Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport C++20 concepts to C++17 in the 'sparqlExpressions' module #1787

Merged
Merged
12 changes: 11 additions & 1 deletion src/backports/concepts.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,16 @@
// `CPP_lambda(capture)(arg)(requires ...)`: Expands lambda to use
// `requires` in C++20 mode and `std::enable_if_t` in C++17 mode.
//
// `CPP_lambda_mut(capture)(arg)(requires ...)`: Same as
// `CPP_lambda` but for mutable lambdas.
//
// `CPP_template_lambda(capture)(typenames...)(arg)(requires ...)`: Expands
// lambda with (C++20) explicit gemplate parameters to use
// lambda with (C++20) explicit template parameters to use
// `requires` in C++20 mode and `std::enable_if_t` in C++17 mode.
//
// `CPP_template_lambda_mut(capture)(typenames...)(arg)(requires ...)`: Same as
// `CPP_template_lambda` but for mutable lambdas.
//
// Example usages:
//
// `QL_CONCEPT_OR_NOTHING(std::view) auto x = someFunction();`
Expand Down Expand Up @@ -63,6 +69,8 @@
#define CPP_member_def CPP_member_def_sfinae
#define CPP_lambda CPP_lambda_sfinae
#define CPP_template_lambda CPP_template_lambda_sfinae
#define CPP_lambda_mut CPP_lambda_mut_sfinae
#define CPP_template_lambda_mut CPP_template_lambda_mut_sfinae
#else
#define QL_CONCEPT_OR_NOTHING(...) __VA_ARGS__
#define QL_CONCEPT_OR_TYPENAME(...) __VA_ARGS__
Expand All @@ -74,6 +82,8 @@
#define CPP_member_def CPP_member
#define CPP_lambda CPP_LAMBDA_20
#define CPP_template_lambda CPP_TEMPLATE_LAMBDA_20
#define CPP_lambda_mut CPP_lambda_mut_20
#define CPP_template_lambda_mut CPP_TEMPLATE_LAMBDA_MUT_20
#endif

// The namespace `ql::concepts` includes concepts that are contained in the
Expand Down
30 changes: 29 additions & 1 deletion src/backports/cppTemplate2.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,17 @@

#define CPP_TEMPLATE_LAMBDA_20(...) [__VA_ARGS__] CPP_TEMPLATE_LAMBDA_ARGS

#define CPP_TEMPLATE_LAMBDA_MUT_20(...) \
[__VA_ARGS__] CPP_TEMPLATE_LAMBDA_MUT_ARGS

// The internals of the `CPP_lambda` template
#define CPP_LAMBDA_SFINAE_ARGS(...) \
(__VA_ARGS__ CPP_LAMBDA_SFINAE_AUX_

#define CPP_LAMBDA_SFINAE_AUX_WHICH_(FIRST, ...) \
CPP_PP_EVAL(CPP_PP_CHECK, CPP_PP_CAT(CPP_LAMBDA_SFINAE_PROBE_CONCEPT_, FIRST))
CPP_PP_EVAL(CPP_PP_CHECK, FIRST)
// CPP_PP_EVAL(CPP_PP_CHECK, CPP_PP_CAT(CPP_LAMBDA_SFINAE_PROBE_CONCEPT_,
// FIRST))

#define CPP_LAMBDA_SFINAE_AUX_(...) \
CPP_PP_CAT(CPP_LAMBDA_SFINAE_AUX_, \
Expand All @@ -87,9 +92,15 @@ CPP_PP_CAT(CPP_LAMBDA_SFINAE_AUX_3_, __VA_ARGS__) \
#define CPP_TEMPLATE_LAMBDA_ARGS_sfinae(...) \
<__VA_ARGS__> CPP_LAMBDA_SFINAE_ARGS

#define CPP_TEMPLATE_LAMBDA_MUT_ARGS_sfinae(...) \
<__VA_ARGS__> CPP_LAMBDA_MUT_SFINAE_ARGS

#define CPP_template_lambda_sfinae(...) \
[__VA_ARGS__] CPP_TEMPLATE_LAMBDA_ARGS_sfinae

#define CPP_template_lambda_mut_sfinae(...) \
[__VA_ARGS__] CPP_TEMPLATE_LAMBDA_MUT_ARGS_sfinae

#define CPP_LAMBDA_SFINAE_AUX_3_requires

#define CPP_LAMBDA_ARGS(...) (__VA_ARGS__) CPP_LAMBDA_AUX_
Expand All @@ -102,3 +113,20 @@ CPP_PP_CAT(CPP_LAMBDA_SFINAE_AUX_3_, __VA_ARGS__) \
#define CPP_LAMBDA_AUX_0(...) __VA_ARGS__

#define CPP_TEMPLATE_LAMBDA_ARGS(...) <__VA_ARGS__> CPP_LAMBDA_ARGS

#define CPP_TEMPLATE_LAMBDA_MUT_ARGS(...) <__VA_ARGS__> CPP_LAMBDA_ARGS_MUT

#define CPP_lambda_mut_sfinae(...) \
CPP_PP_IGNORE_CXX2A_COMPAT_BEGIN \
[__VA_ARGS__] CPP_LAMBDA_MUT_SFINAE_ARGS

#define CPP_LAMBDA_MUT_SFINAE_ARGS(...) \
(__VA_ARGS__ CPP_LAMBDA_MUT_SFINAE_AUX_

#define CPP_LAMBDA_MUT_SFINAE_AUX_(...) \
CPP_PP_CAT(CPP_LAMBDA_SFINAE_AUX_, \
CPP_LAMBDA_SFINAE_AUX_WHICH_(__VA_ARGS__, )) \
(__VA_ARGS__) mutable

#define CPP_LAMBDA_ARGS_MUT(...) (__VA_ARGS__) mutable CPP_LAMBDA_AUX_
#define CPP_lambda_mut_20(...) [__VA_ARGS__] CPP_LAMBDA_ARGS_MUT
7 changes: 3 additions & 4 deletions src/engine/Bind.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ IdTable Bind::computeExpressionBind(
idTable.addEmptyColumn();
auto outputColumn = idTable.getColumn(idTable.numColumns() - 1);

auto visitor = [&]<sparqlExpression::SingleExpressionResult T>(
T&& singleResult) mutable {
auto visitor = CPP_template_lambda_mut(&)(typename T)(T && singleResult)(
requires sparqlExpression::SingleExpressionResult<T>) {
constexpr static bool isVariable = std::is_same_v<T, ::Variable>;
constexpr static bool isStrongId = std::is_same_v<T, Id>;

Expand All @@ -180,8 +180,7 @@ IdTable Bind::computeExpressionBind(
constexpr bool isConstant = sparqlExpression::isConstantResult<T>;

auto resultGenerator = sparqlExpression::detail::makeGenerator(
std::forward<T>(singleResult), outputColumn.size(),
&evaluationContext);
AD_FWD(singleResult), outputColumn.size(), &evaluationContext);

if constexpr (isConstant) {
auto it = resultGenerator.begin();
Expand Down
123 changes: 60 additions & 63 deletions src/engine/Filter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,70 +147,67 @@ CPP_template_def(int WIDTH, typename Table)(
// NOTE: the explicit (seemingly redundant) capture of `resultTable` is
// required to work around a bug in Clang 17, see
// https://github.com/llvm/llvm-project/issues/61267
auto computeResult =
[this, &resultTable = resultTable, &input, &inputTable,
&dynamicResultTable,
&evaluationContext]<sparqlExpression::SingleExpressionResult T>(
T&& singleResult) {
if constexpr (std::is_same_v<T, ad_utility::SetOfIntervals>) {
AD_CONTRACT_CHECK(input.size() == evaluationContext.size());
// If the expression result is given as a set of intervals, we copy
// the corresponding parts of `input` to `resultTable`.
//
// NOTE: One of the interval ends may be larger than `input.size()`
// (as the result of a negation).
auto totalSize = std::accumulate(
singleResult._intervals.begin(), singleResult._intervals.end(),
resultTable.size(),
[&input](const auto& sum, const auto& interval) {
size_t intervalBegin = interval.first;
size_t intervalEnd = std::min(interval.second, input.size());
return sum + (intervalEnd - intervalBegin);
});
if (resultTable.empty() && totalSize == inputTable.size()) {
// The binary filter contains all elements of the input, and we have
// no previous results, so we can simply copy or move the complete
// table.
dynamicResultTable = AD_FWD(inputTable).moveOrClone();
return;
}
checkCancellation();
for (auto [intervalBegin, intervalEnd] : singleResult._intervals) {
intervalEnd = std::min(intervalEnd, input.size());
resultTable.insertAtEnd(inputTable, intervalBegin, intervalEnd);
checkCancellation();
}
AD_CORRECTNESS_CHECK(resultTable.size() == totalSize);
} else {
// In the general case, we generate all expression results and apply
// the `EffectiveBooleanValueGetter` to each.
//
// NOTE: According to the standard, this means that values like zero,
// UNDEF, and empty strings are converted to `false` and hence the
// corresponding rows from `input` are filtered out.
//
// TODO<joka921> Check whether it is feasible to precompute the
// number of `true` values and use that to reserve the right
// amount of space for `resultTable`, like we do it for the set of
// intervals above. This depends on how expensive the evaluation with
// the `EffectiveBooleanValueGetter` is.
auto resultGenerator = sparqlExpression::detail::makeGenerator(
std::forward<T>(singleResult), input.size(), &evaluationContext);
size_t i = 0;

using ValueGetter =
sparqlExpression::detail::EffectiveBooleanValueGetter;
ValueGetter valueGetter{};
for (auto&& resultValue : resultGenerator) {
if (valueGetter(resultValue, &evaluationContext) ==
ValueGetter::Result::True) {
resultTable.push_back(input[i]);
}
checkCancellation();
++i;
}
auto computeResult = CPP_template_lambda(
this, &resultTable = resultTable, &input, &inputTable,
&dynamicResultTable, &evaluationContext)(typename T)(T && singleResult)(
requires sparqlExpression::SingleExpressionResult<T>) {
if constexpr (std::is_same_v<T, ad_utility::SetOfIntervals>) {
AD_CONTRACT_CHECK(input.size() == evaluationContext.size());
// If the expression result is given as a set of intervals, we copy
// the corresponding parts of `input` to `resultTable`.
//
// NOTE: One of the interval ends may be larger than `input.size()`
// (as the result of a negation).
auto totalSize = std::accumulate(
singleResult._intervals.begin(), singleResult._intervals.end(),
resultTable.size(), [&input](const auto& sum, const auto& interval) {
size_t intervalBegin = interval.first;
size_t intervalEnd = std::min(interval.second, input.size());
return sum + (intervalEnd - intervalBegin);
});
if (resultTable.empty() && totalSize == inputTable.size()) {
// The binary filter contains all elements of the input, and we have
// no previous results, so we can simply copy or move the complete
// table.
dynamicResultTable = AD_FWD(inputTable).moveOrClone();
return;
}
checkCancellation();
for (auto [intervalBegin, intervalEnd] : singleResult._intervals) {
intervalEnd = std::min(intervalEnd, input.size());
resultTable.insertAtEnd(inputTable, intervalBegin, intervalEnd);
checkCancellation();
}
AD_CORRECTNESS_CHECK(resultTable.size() == totalSize);
} else {
// In the general case, we generate all expression results and apply
// the `EffectiveBooleanValueGetter` to each.
//
// NOTE: According to the standard, this means that values like zero,
// UNDEF, and empty strings are converted to `false` and hence the
// corresponding rows from `input` are filtered out.
//
// TODO<joka921> Check whether it is feasible to precompute the
// number of `true` values and use that to reserve the right
// amount of space for `resultTable`, like we do it for the set of
// intervals above. This depends on how expensive the evaluation with
// the `EffectiveBooleanValueGetter` is.
auto resultGenerator = sparqlExpression::detail::makeGenerator(
AD_FWD(singleResult), input.size(), &evaluationContext);
size_t i = 0;

using ValueGetter = sparqlExpression::detail::EffectiveBooleanValueGetter;
ValueGetter valueGetter{};
for (auto&& resultValue : resultGenerator) {
if (valueGetter(resultValue, &evaluationContext) ==
ValueGetter::Result::True) {
resultTable.push_back(input[i]);
}
};
checkCancellation();
++i;
}
}
};
std::visit(computeResult, std::move(expressionResult));

// Detect the case that we have directly written the `dynamicResultTable`
Expand Down
21 changes: 10 additions & 11 deletions src/engine/GroupBy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,8 @@ void GroupBy::processGroup(
evaluationContext._previousResultsFromSameGroup.at(resultColumn) =
sparqlExpression::copyExpressionResult(expressionResult);

auto visitor = [&]<sparqlExpression::SingleExpressionResult T>(
T&& singleResult) mutable {
auto visitor = CPP_template_lambda_mut(&)(typename T)(T && singleResult)(
requires sparqlExpression::SingleExpressionResult<T>) {
constexpr static bool isStrongId = std::is_same_v<T, Id>;
AD_CONTRACT_CHECK(sparqlExpression::isConstantResult<T>);
if constexpr (isStrongId) {
Expand Down Expand Up @@ -1097,12 +1097,11 @@ void GroupBy::extractValues(
sparqlExpression::ExpressionResult&& expressionResult,
sparqlExpression::EvaluationContext& evaluationContext,
IdTable* resultTable, LocalVocab* localVocab, size_t outCol) {
auto visitor = [&evaluationContext, &resultTable, &localVocab,
&outCol]<sparqlExpression::SingleExpressionResult T>(
T&& singleResult) mutable {
auto visitor = CPP_template_lambda_mut(&evaluationContext, &resultTable,
&localVocab, &outCol)(typename T)(
T && singleResult)(requires sparqlExpression::SingleExpressionResult<T>) {
auto generator = sparqlExpression::detail::makeGenerator(
std::forward<T>(singleResult), evaluationContext.size(),
&evaluationContext);
AD_FWD(singleResult), evaluationContext.size(), &evaluationContext);

auto targetIterator =
resultTable->getColumn(outCol).begin() + evaluationContext._beginIndex;
Expand Down Expand Up @@ -1480,10 +1479,10 @@ static constexpr auto makeProcessGroupsVisitor =
[](size_t blockSize,
const sparqlExpression::EvaluationContext* evaluationContext,
const std::vector<size_t>& hashEntries) {
return [blockSize, evaluationContext,
&hashEntries]<sparqlExpression::SingleExpressionResult T,
VectorOfAggregationData A>(
T&& singleResult, A& aggregationDataVector) {
return CPP_template_lambda(blockSize, evaluationContext, &hashEntries)(
typename T, typename A)(T && singleResult, A & aggregationDataVector)(
requires sparqlExpression::SingleExpressionResult<T> &&
VectorOfAggregationData<A>) {
auto generator = sparqlExpression::detail::makeGenerator(
std::forward<T>(singleResult), blockSize, evaluationContext);

Expand Down
9 changes: 5 additions & 4 deletions src/engine/LazyGroupBy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,12 @@ void LazyGroupBy::processBlock(
evaluationContext);

visitAggregate(
[blockSize,
&evaluationContext]<sparqlExpression::SingleExpressionResult T>(
VectorOfAggregationData auto& aggregateData, T&& singleResult) {
CPP_template_lambda(blockSize, &evaluationContext)(
typename A, typename T)(A & aggregateData, T && singleResult)(
requires VectorOfAggregationData<A> &&
sparqlExpression::SingleExpressionResult<T>) {
auto generator = sparqlExpression::detail::makeGenerator(
std::forward<T>(singleResult), blockSize, &evaluationContext);
AD_FWD(singleResult), blockSize, &evaluationContext);

for (const auto& val : generator) {
aggregateData.at(0).addValue(val, &evaluationContext);
Expand Down
19 changes: 10 additions & 9 deletions src/engine/sparqlExpressions/AggregateExpression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@ namespace sparqlExpression::detail {
// function.
template <typename AggregateOperation, typename FinalOperation>
struct EvaluateOnChildOperand {
ExpressionResult operator()(const AggregateOperation& aggregateOperation,
ValueId resultForEmptyGroup,
const FinalOperation& finalOperation,
EvaluationContext* context, bool distinct,
SingleExpressionResult auto&& operand) const {
template <typename O>
auto operator()(const AggregateOperation& aggregateOperation,
ValueId resultForEmptyGroup,
const FinalOperation& finalOperation,
EvaluationContext* context, bool distinct, O&& operand) const
-> CPP_ret(ExpressionResult)(requires SingleExpressionResult<O>) {
// Perform the more efficient calculation on `SetOfInterval`s if it is
// possible.
//
Expand Down Expand Up @@ -55,9 +56,9 @@ struct EvaluateOnChildOperand {
// Helper lambda for aggregating two values.
auto aggregateTwoValues = [&aggregateOperation, context](
auto&& x, auto&& y) -> decltype(auto) {
if constexpr (requires {
aggregateOperation._function(AD_FWD(x), AD_FWD(y));
}) {
if constexpr (ranges::invocable<decltype(aggregateOperation._function),
decltype(AD_FWD(x)),
decltype(AD_FWD(y))>) {
Comment on lines +59 to +61
Copy link
Member

Choose a reason for hiding this comment

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

If you give the X and Y names in the lambda declaration, then this is
ranges::invocable<AggregateOperation, X&&, Y&&>

This is more readable then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean X, and Y as lambda template parameters?
However, the problem is that the lambda explicit template parameter list has been available since C++20.
So later we need to somehow deal with this to be compatible with C++17.

return aggregateOperation._function(AD_FWD(x), AD_FWD(y));
} else {
return aggregateOperation._function(AD_FWD(x), AD_FWD(y), context);
Expand Down Expand Up @@ -117,7 +118,7 @@ struct EvaluateOnChildOperand {
//
// TODO<joka921> Check if this is really necessary, or if we can also use
// IDs in the intermediate steps without loss of efficiency.
if constexpr (requires { makeNumericId(result); }) {
if constexpr (ValueAsNumericId<decltype(result)>) {
return makeNumericId(result);
} else {
return result;
Expand Down
30 changes: 16 additions & 14 deletions src/engine/sparqlExpressions/AggregateExpression.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,9 @@ using AGG_EXP = AggregateExpression<
// with arguments and result of type `NumericValue` (which is a `std::variant`).
template <typename NumericOperation>
inline auto makeNumericExpressionForAggregate() {
return [](const std::same_as<NumericValue> auto&... args) -> NumericValue {
return []<typename... Args>(const Args&... args)
-> CPP_ret(NumericValue)(
requires(concepts::same_as<Args, NumericValue>&&...)) {
auto visitor = []<typename... Ts>(const Ts&... t) -> NumericValue {
if constexpr ((... || std::is_same_v<NotNumeric, Ts>)) {
return NotNumeric{};
Expand Down Expand Up @@ -181,19 +183,19 @@ inline const auto compareIdsOrStrings =

// Aggregate expression for MIN and MAX.
template <valueIdComparators::Comparison comparison>
inline const auto minMaxLambdaForAllTypes =
[]<SingleExpressionResult T>(const T& a, const T& b,
const EvaluationContext* ctx) {
auto actualImpl = [ctx](const auto& x, const auto& y) {
return compareIdsOrStrings<comparison>(x, y, ctx);
};
if constexpr (ad_utility::isSimilar<T, Id>) {
return std::get<Id>(actualImpl(a, b));
} else {
// TODO<joka921> We should definitely move strings here.
return std::visit(actualImpl, a, b);
}
};
inline const auto minMaxLambdaForAllTypes = CPP_template_lambda()(typename T)(
const T& a, const T& b,
const EvaluationContext* ctx)(requires SingleExpressionResult<T>) {
auto actualImpl = [ctx](const auto& x, const auto& y) {
return compareIdsOrStrings<comparison>(x, y, ctx);
};
if constexpr (ad_utility::isSimilar<T, Id>) {
return std::get<Id>(actualImpl(a, b));
} else {
// TODO<joka921> We should definitely move strings here.
return std::visit(actualImpl, a, b);
}
};
constexpr inline auto minLambdaForAllTypes =
minMaxLambdaForAllTypes<valueIdComparators::Comparison::LT>;
constexpr inline auto maxLambdaForAllTypes =
Expand Down
Loading
Loading