Consistently replace std::ranges by ql::ranges

[gpicciuca]

When building with -DUSE_CPP_17_BACKPORT=ON, then range-v3 is used instead of std::ranges in all places (types, functions, concepts).

[joka921]

Thank you very much for that thorough work!
I only have a single wish remaining, you can see it from the comment.
I am planning to update the range-v3 library s.t. the intefaces for unique, remove_if and partition are consistent, but I like the workarounds you chose .

I think if you quickly update the one change and all the checks run through, then we can merge this soon.

[joka921]

Can you put them back into the .cpp file?
(we prefer the .cpp files wherever feasible, because QLever's compile times are already long enough as they are:)

the
template <typename R> requires(....) then has to stand before both the declaration in the header and the definition in the implementation file.

[gpicciuca]

Thank you for your feedback. Will address it shortly.

[joka921]

Hi,
Two comments:

1. After an initial review has been done on GitHub please do not rebase/force-push, but rather just add the commits you add. Otherwise the diff on GitHub is broken (I cannot see what has been changed since the last review anymore) which is especially an issue for large PRs.

2. As far as I understand, the CPP_template macro doesn't work for separate declaration + definition of a template in C++17 mode.
   There are currently two ways around that (for the two functions in CartesianProductJoin:

• Directly define them in the header (which I have discouraged in the previous review).
• Use the macros in backports/Concepts.h that completely disable the concepts in C++17 mode. They should work in this case. So I would suggest (in the header as well as the .cpp file

template <QL_CONCEPT_OR_TYPENAME(ql::ranges::random_access_range) R 
IdTable writeAllColumns(R idTables, ....);

With this change, all checks should run through and we can merge this.
Best regards
Johannes

[gpicciuca]

Hi, Two comments:

1. After an initial review has been done on GitHub please do not rebase/force-push, but rather just add the commits you add. Otherwise the diff on GitHub is broken (I cannot see what has been changed since the last review anymore) which is especially an issue for large PRs.
2. As far as I understand, the CPP_template macro doesn't work for separate declaration + definition of a template in C++17 mode.
   There are currently two ways around that (for the two functions in CartesianProductJoin:

• Directly define them in the header (which I have discouraged in the previous review).
• Use the macros in backports/Concepts.h that completely disable the concepts in C++17 mode. They should work in this case. So I would suggest (in the header as well as the .cpp file

template <QL_CONCEPT_OR_TYPENAME(ql::ranges::random_access_range) R 
IdTable writeAllColumns(R idTables, ....);

With this change, all checks should run through and we can merge this. Best regards Johannes

Hello,

1. My apologies for that.
2. Applied the proposed approach. Experimented a bit on godbolt, but there seems to be no way to get it to work in C++17. CPP_template gives redefinition errors and manually writing the whole template with std::enable_if makes it fail to deduce the datatypes.

BR,
Giuseppe

[joka921]

2. Applied the proposed approach. Experimented a bit on godbolt, but there seems to be no way to get it to work in C++17. CPP_template gives redefinition errors and manually writing the whole template with std::enable_if makes it fail to deduce the datatypes.

BR, Giuseppe

Thank you, I will wait for the checks to go through and then fix them.

Just for some context and information:
You are correct, I haven't yet found a good solution for the template rewriting when we have separate declarations and definitions.
The std::enable_if would work, so e.g.

// declaration
template <typename R, typename = std::enable_if_t<ql::random_access_range<R>>
IdTable doSomething();
// Definition
template <typename R, typename>
IdTable doSomething() {}

But I would prefer a solution that can still do concepts in C++20 mode, but I have to fiddle with the macros a bit.

[joka921]

One + one final thing

1. For following PRs, please don't use the master branch of your PR, but give it a distinct name. Otherwise it is really hard for other people to locally checkout and play with the PR, as the branch names always collide, which git doesn't like that much.

2. I found the solution for the CPP_template thin, and will also shortly update it in the Wiki:

When you have a separater declaration + definition, your write

CPP_template(typename T) (requires something<T>) void f();
And then for the definition (or redeclaration etc.) you use the CPP_template_def macro, so

CPP_template_def(typename T) (requires something<T>) void f() {....}.

Can you please add this to the CartesianProductJoin functions (I would have done it myself, but I wasn't motivated to work around the issue of 1.

Best regards
Johannes

[sparql-conformance]

Conformance check passed ✅

No test result changes.

Details: https://qlever.cs.uni-freiburg.de/sparql-conformance-ui?cur=466a469db82dd8211735b8c6179ba61f6b0c38a8&prev=ff479223aa6d7597e886b3fee24f1167e643cf5e

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.91%. Comparing base (d7ec9be) to head (466a469).
Report is 7 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1729      +/-   ##
==========================================
+ Coverage   89.90%   89.91%   +0.01%     
==========================================
  Files         393      393              
  Lines       37479    37620     +141     
  Branches     4221     4231      +10     
==========================================
+ Hits        33694    33826     +132     
- Misses       2482     2494      +12     
+ Partials     1303     1300       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud