From 5251f9ee86b15ed8fdf75016242e3e856a126cdc Mon Sep 17 00:00:00 2001 From: Krzysztof Tyburski <196668810+krzysztof-tyb@users.noreply.github.com> Date: Fri, 14 Feb 2025 14:29:46 +0100 Subject: [PATCH] Use CPP_template_lambda and CPP_template_lambda_mut --- src/backports/concepts.h | 10 +++- src/backports/cppTemplate2.h | 11 ++++ src/engine/Bind.cpp | 14 ++---- src/engine/Filter.cpp | 12 ++--- src/engine/GroupBy.cpp | 50 ++++++++----------- src/engine/LazyGroupBy.cpp | 10 ++-- .../sparqlExpressions/AggregateExpression.h | 31 ++++++------ .../ConditionalExpressions.cpp | 27 +++++----- .../SparqlExpressionGenerators.h | 12 ++--- .../sparqlExpressions/StringExpressions.cpp | 8 +-- 10 files changed, 93 insertions(+), 92 deletions(-) diff --git a/src/backports/concepts.h b/src/backports/concepts.h index 5873f7d0ea..15a8897047 100644 --- a/src/backports/concepts.h +++ b/src/backports/concepts.h @@ -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();` @@ -64,6 +70,7 @@ #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__ @@ -76,6 +83,7 @@ #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 diff --git a/src/backports/cppTemplate2.h b/src/backports/cppTemplate2.h index e250034f19..830d9e7e21 100644 --- a/src/backports/cppTemplate2.h +++ b/src/backports/cppTemplate2.h @@ -63,6 +63,9 @@ #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_ @@ -89,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_ @@ -105,6 +114,8 @@ CPP_PP_CAT(CPP_LAMBDA_SFINAE_AUX_3_, __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 diff --git a/src/engine/Bind.cpp b/src/engine/Bind.cpp index 86b9ded990..96e5d5edf6 100644 --- a/src/engine/Bind.cpp +++ b/src/engine/Bind.cpp @@ -161,13 +161,10 @@ IdTable Bind::computeExpressionBind( idTable.addEmptyColumn(); auto outputColumn = idTable.getColumn(idTable.numColumns() - 1); - auto visitor = CPP_lambda_mut(&)(auto&& singleResult)( - requires sparqlExpression::SingleExpressionResult< - std::decay_t>) { - constexpr static bool isVariable = - std::is_same_v, ::Variable>; - constexpr static bool isStrongId = - std::is_same_v, Id>; + auto visitor = CPP_template_lambda_mut(&)(typename T)(T && singleResult)( + requires sparqlExpression::SingleExpressionResult) { + constexpr static bool isVariable = std::is_same_v; + constexpr static bool isStrongId = std::is_same_v; if constexpr (isVariable) { auto columnIndex = @@ -180,8 +177,7 @@ IdTable Bind::computeExpressionBind( ad_utility::chunkedFill(outputColumn, singleResult, CHUNK_SIZE, [this]() { checkCancellation(); }); } else { - constexpr bool isConstant = - sparqlExpression::isConstantResult; + constexpr bool isConstant = sparqlExpression::isConstantResult; auto resultGenerator = sparqlExpression::detail::makeGenerator( AD_FWD(singleResult), outputColumn.size(), &evaluationContext); diff --git a/src/engine/Filter.cpp b/src/engine/Filter.cpp index 0e09d2ef9b..fe5c6ad3e0 100644 --- a/src/engine/Filter.cpp +++ b/src/engine/Filter.cpp @@ -147,13 +147,11 @@ 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 = - CPP_lambda(this, &resultTable = resultTable, &input, &inputTable, - &dynamicResultTable, &evaluationContext)(auto&& singleResult)( - requires sparqlExpression::SingleExpressionResult< - std::decay_t>) { - if constexpr (std::is_same_v, - ad_utility::SetOfIntervals>) { + auto computeResult = CPP_template_lambda( + this, &resultTable = resultTable, &input, &inputTable, + &dynamicResultTable, &evaluationContext)(typename T)(T && singleResult)( + requires sparqlExpression::SingleExpressionResult) { + if constexpr (std::is_same_v) { 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`. diff --git a/src/engine/GroupBy.cpp b/src/engine/GroupBy.cpp index 259ec703ab..5205e4723e 100644 --- a/src/engine/GroupBy.cpp +++ b/src/engine/GroupBy.cpp @@ -208,17 +208,13 @@ void GroupBy::processGroup( evaluationContext._previousResultsFromSameGroup.at(resultColumn) = sparqlExpression::copyExpressionResult(expressionResult); - auto visitor = CPP_lambda_mut(&)(auto&& singleResult)( - requires sparqlExpression::SingleExpressionResult< - std::decay_t>) { - constexpr static bool isStrongId = - std::is_same_v, Id>; - AD_CONTRACT_CHECK(sparqlExpression::isConstantResult< - std::decay_t>); + auto visitor = CPP_template_lambda_mut(&)(typename T)(T && singleResult)( + requires sparqlExpression::SingleExpressionResult) { + constexpr static bool isStrongId = std::is_same_v; + AD_CONTRACT_CHECK(sparqlExpression::isConstantResult); if constexpr (isStrongId) { resultEntry = singleResult; - } else if constexpr (sparqlExpression::isConstantResult< - std::decay_t>) { + } else if constexpr (sparqlExpression::isConstantResult) { resultEntry = sparqlExpression::detail::constantExpressionResultToId( AD_FWD(singleResult), *localVocab); } else { @@ -1101,10 +1097,9 @@ void GroupBy::extractValues( sparqlExpression::ExpressionResult&& expressionResult, sparqlExpression::EvaluationContext& evaluationContext, IdTable* resultTable, LocalVocab* localVocab, size_t outCol) { - auto visitor = CPP_lambda_mut(&evaluationContext, &resultTable, &localVocab, - &outCol)(auto&& singleResult)( - requires sparqlExpression::SingleExpressionResult< - std::decay_t>) { + auto visitor = CPP_template_lambda_mut(&evaluationContext, &resultTable, + &localVocab, &outCol)(typename T)( + T && singleResult)(requires sparqlExpression::SingleExpressionResult) { auto generator = sparqlExpression::detail::makeGenerator( AD_FWD(singleResult), evaluationContext.size(), &evaluationContext); @@ -1484,25 +1479,24 @@ static constexpr auto makeProcessGroupsVisitor = [](size_t blockSize, const sparqlExpression::EvaluationContext* evaluationContext, const std::vector& hashEntries) { - return - [blockSize, evaluationContext, &hashEntries]( - T&& singleResult, A& aggregationDataVector) { - CPP_assert(sparqlExpression::SingleExpressionResult); - CPP_assert(VectorOfAggregationData); - auto generator = sparqlExpression::detail::makeGenerator( - std::forward(singleResult), blockSize, evaluationContext); + return CPP_template_lambda(blockSize, evaluationContext, &hashEntries)( + typename T, typename A)(T && singleResult, A & aggregationDataVector)( + requires sparqlExpression::SingleExpressionResult && + VectorOfAggregationData) { + auto generator = sparqlExpression::detail::makeGenerator( + std::forward(singleResult), blockSize, evaluationContext); - auto hashEntryIndex = 0; + auto hashEntryIndex = 0; - for (const auto& val : generator) { - auto vectorOffset = hashEntries[hashEntryIndex]; - auto& aggregateData = aggregationDataVector.at(vectorOffset); + for (const auto& val : generator) { + auto vectorOffset = hashEntries[hashEntryIndex]; + auto& aggregateData = aggregationDataVector.at(vectorOffset); - aggregateData.addValue(val, evaluationContext); + aggregateData.addValue(val, evaluationContext); - ++hashEntryIndex; - } - }; + ++hashEntryIndex; + } + }; }; // _____________________________________________________________________________ diff --git a/src/engine/LazyGroupBy.cpp b/src/engine/LazyGroupBy.cpp index 572d1bbb0d..60a1249e69 100644 --- a/src/engine/LazyGroupBy.cpp +++ b/src/engine/LazyGroupBy.cpp @@ -73,12 +73,10 @@ void LazyGroupBy::processBlock( evaluationContext); visitAggregate( - CPP_lambda(blockSize, &evaluationContext)(auto& aggregateData, - auto&& singleResult)( - requires VectorOfAggregationData< - std::decay_t>> && - sparqlExpression::SingleExpressionResult< - std::decay_t>) { + CPP_template_lambda(blockSize, &evaluationContext)( + typename A, typename T)(A & aggregateData, T && singleResult)( + requires VectorOfAggregationData && + sparqlExpression::SingleExpressionResult) { auto generator = sparqlExpression::detail::makeGenerator( AD_FWD(singleResult), blockSize, &evaluationContext); diff --git a/src/engine/sparqlExpressions/AggregateExpression.h b/src/engine/sparqlExpressions/AggregateExpression.h index fb9d553219..b43827ddb3 100644 --- a/src/engine/sparqlExpressions/AggregateExpression.h +++ b/src/engine/sparqlExpressions/AggregateExpression.h @@ -104,10 +104,9 @@ using AGG_EXP = AggregateExpression< // with arguments and result of type `NumericValue` (which is a `std::variant`). template inline auto makeNumericExpressionForAggregate() { - return [](const auto&... args) + return [](const Args&... args) -> CPP_ret(NumericValue)( - requires(concepts::same_as, - NumericValue>&&...)) { + requires(concepts::same_as&&...)) { auto visitor = [](const Ts&... t) -> NumericValue { if constexpr ((... || std::is_same_v)) { return NotNumeric{}; @@ -184,19 +183,19 @@ inline const auto compareIdsOrStrings = // Aggregate expression for MIN and MAX. template -inline const auto minMaxLambdaForAllTypes = - [](const T& a, const T& b, const EvaluationContext* ctx) { - CPP_assert(SingleExpressionResult); - auto actualImpl = [ctx](const auto& x, const auto& y) { - return compareIdsOrStrings(x, y, ctx); - }; - if constexpr (ad_utility::isSimilar) { - return std::get(actualImpl(a, b)); - } else { - // TODO 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) { + auto actualImpl = [ctx](const auto& x, const auto& y) { + return compareIdsOrStrings(x, y, ctx); + }; + if constexpr (ad_utility::isSimilar) { + return std::get(actualImpl(a, b)); + } else { + // TODO We should definitely move strings here. + return std::visit(actualImpl, a, b); + } +}; constexpr inline auto minLambdaForAllTypes = minMaxLambdaForAllTypes; constexpr inline auto maxLambdaForAllTypes = diff --git a/src/engine/sparqlExpressions/ConditionalExpressions.cpp b/src/engine/sparqlExpressions/ConditionalExpressions.cpp index ae988f7ab2..bc26000d20 100644 --- a/src/engine/sparqlExpressions/ConditionalExpressions.cpp +++ b/src/engine/sparqlExpressions/ConditionalExpressions.cpp @@ -65,11 +65,10 @@ class CoalesceExpression : public VariadicExpression { std::get(x) == Id::makeUndefined()); }; - auto visitConstantExpressionResult = CPP_lambda( - &nextUnboundIndices, &unboundIndices, &isUnbound, &result, - ctx)(auto&& childResult)( - requires SingleExpressionResult> && - isConstantResult>) { + auto visitConstantExpressionResult = + CPP_template_lambda(&nextUnboundIndices, &unboundIndices, &isUnbound, + &result, ctx)(typename T)(T && childResult)( + requires SingleExpressionResult && isConstantResult) { IdOrLiteralOrIri constantResult{AD_FWD(childResult)}; if (isUnbound(constantResult)) { nextUnboundIndices = std::move(unboundIndices); @@ -94,11 +93,10 @@ class CoalesceExpression : public VariadicExpression { // result so far is unbound, and the child result is bound. While doing so, // set up the `nextUnboundIndices` vector for the next step. auto visitVectorExpressionResult = - CPP_lambda(&result, &unboundIndices, &nextUnboundIndices, &ctx, - &isUnbound)(auto&& childResult)(requires CPP_NOT( - isConstantResult> && - SingleExpressionResult> && - std::is_rvalue_reference_v)) { + CPP_template_lambda(&result, &unboundIndices, &nextUnboundIndices, &ctx, + &isUnbound)(typename T)(T && childResult)( + requires CPP_NOT(isConstantResult && SingleExpressionResult && + std::is_rvalue_reference_v)) { auto gen = detail::makeGenerator(AD_FWD(childResult), ctx->size(), ctx); // Iterator to the next index where the result so far is unbound. auto unboundIdxIt = unboundIndices.begin(); @@ -128,14 +126,13 @@ class CoalesceExpression : public VariadicExpression { }, [ctx]() { ctx->cancellationHandle_->throwIfCancelled(); }); }; - auto visitExpressionResult = CPP_lambda( + auto visitExpressionResult = CPP_template_lambda( &visitConstantExpressionResult, - &visitVectorExpressionResult)(auto&& childResult)( - requires SingleExpressionResult> && - std::is_rvalue_reference_v) { + &visitVectorExpressionResult)(typename T)(T && childResult)( + requires SingleExpressionResult && std::is_rvalue_reference_v) { // If the previous expression result is a constant, we can skip the // loop. - if constexpr (isConstantResult>) { + if constexpr (isConstantResult) { visitConstantExpressionResult(AD_FWD(childResult)); } else { visitVectorExpressionResult(AD_FWD(childResult)); diff --git a/src/engine/sparqlExpressions/SparqlExpressionGenerators.h b/src/engine/sparqlExpressions/SparqlExpressionGenerators.h index 22a89148f3..3f317bb384 100644 --- a/src/engine/sparqlExpressions/SparqlExpressionGenerators.h +++ b/src/engine/sparqlExpressions/SparqlExpressionGenerators.h @@ -106,12 +106,12 @@ CPP_template(typename Input, typename Transformation = std::identity)( /// Generate `numItems` many values from the `input` and apply the /// `valueGetter` to each of the values. inline auto valueGetterGenerator = - CPP_lambda()(size_t numElements, EvaluationContext* context, auto&& input, - auto&& valueGetter)( - requires SingleExpressionResult>) { - auto transformation = CPP_lambda(context, valueGetter)(auto&& i)( - requires ranges::invocable, - decltype(i), EvaluationContext*>) { + CPP_template_lambda()(typename ValueGetter, typename Input)( + size_t numElements, EvaluationContext* context, Input&& input, + ValueGetter&& valueGetter)(requires SingleExpressionResult) { + auto transformation = + CPP_template_lambda(context, valueGetter)(typename I)(I && i)( + requires ranges::invocable) { context->cancellationHandle_->throwIfCancelled(); return valueGetter(AD_FWD(i), context); }; diff --git a/src/engine/sparqlExpressions/StringExpressions.cpp b/src/engine/sparqlExpressions/StringExpressions.cpp index bab1907772..899342c5ed 100644 --- a/src/engine/sparqlExpressions/StringExpressions.cpp +++ b/src/engine/sparqlExpressions/StringExpressions.cpp @@ -382,10 +382,10 @@ class ConcatExpression : public detail::VariadicExpression { // If the result is a string, then all the previously evaluated children // were constants (see above). std::variant result{std::string{""}}; - auto visitSingleExpressionResult = CPP_lambda(&ctx, &result)(auto&& s)( - requires SingleExpressionResult> && - std::is_rvalue_reference_v) { - if constexpr (isConstantResult>) { + auto visitSingleExpressionResult = CPP_template_lambda( + &ctx, &result)(typename T)(T && s)(requires SingleExpressionResult && + std::is_rvalue_reference_v) { + if constexpr (isConstantResult) { std::string strFromConstant = StringValueGetter{}(s, ctx).value_or(""); if (std::holds_alternative(result)) { // All previous children were constants, and the current child also is