Skip to content

Yet another round of C++17 hygiene#1469

Merged
trueqbit merged 10 commits intodevfrom
c++17-code-quality
Dec 9, 2025
Merged

Yet another round of C++17 hygiene#1469
trueqbit merged 10 commits intodevfrom
c++17-code-quality

Conversation

@trueqbit
Copy link
Copy Markdown
Collaborator

@trueqbit trueqbit commented Dec 9, 2025

Yet another PR with a bit of C++17 hygiene and a few improvements.

C++17:

  • Collapsed nested namespaces, meaning: namespace sqlite_orm { namespace internal {} } -> namespace sqlite_orm::internal {}. This reduces code intendation, and makes it more obvious (to collaborators) what's part of the public namespace.
    Note: Because this changes a lot of white spacing it is best to hide whitespace for diff comparison.
  • Avoided using standard type trait variable templates: As long as we don't require a C++17 Standard Library base line we cannot assume that variable templates are available.

Improvements:

  • Qualified calls to std::get(). This function is really widely overloaded, so I consider it good practice in template code to qualify it if called with a tuple.
  • Simplified a trait testing for whether an expression is a binary operator.

Fixes:

@trueqbit trueqbit requested a review from fnc12 December 9, 2025 08:31
namespace internal {
namespace sqlite_orm::internal {
template<class... Op>
std::unique_ptr<std::string> column_constraints<Op...>::default_value() const {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: later this can be translated to std::optional instead of std::unique. I used std::unique initially cause C++14 doesn't have std::optional

Copy link
Copy Markdown
Collaborator Author

@trueqbit trueqbit Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I absolutely agree, however as I wrote in the PR description: "As long as we don't require a C++17 Standard Library base line we cannot assume that variable templates are available.". This is also true for all other C++17 Library facilities.

using expression_type = Select;
using explicit_colrefs_tuple = ExplicitCols;
using hints_tuple = Hints;
static constexpr size_t explicit_colref_count = std::tuple_size<ExplicitCols>::value;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we come back from *_v to *::value approach here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I wrote in the PR description: "As long as we don't require a C++17 Standard Library base line we cannot assume that variable templates are available."

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah I see. Pardon, thanks

@trueqbit trueqbit merged commit f16e63b into dev Dec 9, 2025
4 checks passed
@trueqbit trueqbit deleted the c++17-code-quality branch December 9, 2025 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants