From e335872625bc2dc22d86b949a9c728f2121d4bef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Germain?= Date: Thu, 18 Jun 2026 20:52:49 -0700 Subject: [PATCH 1/3] GenEx: generalize bound operands to a frame and add $<_1> Allow a binding operation to bind more than one operand at once, exposed as $<_0>, $<_1>, .... A single value remains the common case, but the upcoming SORT COMPARATOR must bind the two elements being compared, so the binding becomes an indexed frame and $<_1> is added. Referencing an index the active binding does not provide is reported as an error. Issue: #27892 --- Source/cmGenExContext.h | 28 ++++++--- Source/cmGeneratorExpressionNode.cxx | 61 ++++++++++++++----- Tests/CMakeLib/testGenExBoundOperand.cxx | 40 +++++++++++- .../BoundOperand1OutsideBinding-result.txt | 1 + .../BoundOperand1OutsideBinding-stderr.txt | 1 + .../BoundOperand1OutsideBinding.cmake | 4 ++ .../GeneratorExpression/RunCMakeTest.cmake | 1 + 7 files changed, 110 insertions(+), 26 deletions(-) create mode 100644 Tests/RunCMake/GeneratorExpression/BoundOperand1OutsideBinding-result.txt create mode 100644 Tests/RunCMake/GeneratorExpression/BoundOperand1OutsideBinding-stderr.txt create mode 100644 Tests/RunCMake/GeneratorExpression/BoundOperand1OutsideBinding.cmake diff --git a/Source/cmGenExContext.h b/Source/cmGenExContext.h index 14f6fa20b0..491fed7d60 100644 --- a/Source/cmGenExContext.h +++ b/Source/cmGenExContext.h @@ -2,8 +2,10 @@ file LICENSE.rst or https://cmake.org/licensing for details. */ #pragma once +#include #include #include +#include #include @@ -26,26 +28,36 @@ struct Context final void SetCMP0189(cmPolicies::PolicyStatus cmp0189); cmPolicies::PolicyStatus GetCMP0189() const; + void SetBoundOperands(std::vector operands); void SetBoundOperand(std::string value); - bool HasBoundOperand() const; - std::string const& GetBoundOperand() const; + std::size_t BoundOperandCount() const; + bool HasBoundOperand(std::size_t index = 0) const; + std::string const& GetBoundOperand(std::size_t index = 0) const; private: cm::optional CMP0189; - cm::optional BoundOperand; + std::vector BoundOperands; }; +inline void Context::SetBoundOperands(std::vector operands) +{ + this->BoundOperands = std::move(operands); +} inline void Context::SetBoundOperand(std::string value) { - this->BoundOperand = std::move(value); + this->SetBoundOperands({ std::move(value) }); } -inline bool Context::HasBoundOperand() const +inline std::size_t Context::BoundOperandCount() const { - return this->BoundOperand.has_value(); + return this->BoundOperands.size(); } -inline std::string const& Context::GetBoundOperand() const +inline bool Context::HasBoundOperand(std::size_t index) const { - return *this->BoundOperand; + return index < this->BoundOperandCount(); +} +inline std::string const& Context::GetBoundOperand(std::size_t index) const +{ + return this->BoundOperands[index]; } } } diff --git a/Source/cmGeneratorExpressionNode.cxx b/Source/cmGeneratorExpressionNode.cxx index fe6e92594d..28c3e82ca4 100644 --- a/Source/cmGeneratorExpressionNode.cxx +++ b/Source/cmGeneratorExpressionNode.cxx @@ -107,17 +107,17 @@ std::string cmGeneratorExpressionNode::EvaluateDependentExpression( return result; } -// Re-evaluate the unevaluated subtree of a binding operation with -// `$<_0>` bound to the given operand. A fresh Evaluation is built from a -// copied, mutated Context so that nested binding operations can shadow `$<_0>` -// and restore it on exit. -static std::string EvaluateBodyWithBoundOperand( +// Re-evaluate the unevaluated subtree of a binding operation with the +// given operands bound (accessible as $<_0>, $<_1>, ...). A fresh Evaluation +// is built from a copied, mutated Context so that nested binding operations +// can shadow the operands and restore them on exit. +static std::string EvaluateBodyWithBoundOperands( cmGeneratorExpressionEvaluatorVector const& bodyExpr, - std::string const& operand, cm::GenEx::Evaluation* eval, + std::vector operands, cm::GenEx::Evaluation* eval, cmGeneratorExpressionDAGChecker* dagChecker) { cm::GenEx::Context elemContext = eval->Context; // copy - elemContext.SetBoundOperand(operand); + elemContext.SetBoundOperands(std::move(operands)); cm::GenEx::Evaluation elemEval( elemContext, eval->Quiet, eval->HeadTarget, eval->CurrentTarget, eval->EvaluateForBuildsystem, eval->Backtrace); @@ -149,6 +149,15 @@ static std::string EvaluateBodyWithBoundOperand( return result; } +static std::string EvaluateBodyWithBoundOperand( + cmGeneratorExpressionEvaluatorVector const& bodyExpr, + std::string const& operand, cm::GenEx::Evaluation* eval, + cmGeneratorExpressionDAGChecker* dagChecker) +{ + return EvaluateBodyWithBoundOperands(bodyExpr, { operand }, eval, + dagChecker); +} + // Evaluate `predicateBody` once per element of `list`, binding `$<_0>` to the // element (reusing EvaluateBodyWithBoundOperand). Each result must be exactly // "0" or "1". Returns the per-element boolean mask, or cm::nullopt after @@ -220,9 +229,12 @@ static const struct OneNode : public cmGeneratorExpressionNode } } oneNode; -static const struct BoundOperandNode : public cmGeneratorExpressionNode +struct BoundOperandNode : public cmGeneratorExpressionNode { - BoundOperandNode() {} // NOLINT(modernize-use-equals-default) + explicit BoundOperandNode(std::size_t index) + : Index(index) + { + } int NumExpectedParameters() const override { return 0; } @@ -231,15 +243,31 @@ static const struct BoundOperandNode : public cmGeneratorExpressionNode cm::GenEx::Evaluation* eval, GeneratorExpressionContent const* content, cmGeneratorExpressionDAGChecker* /*dagChecker*/) const override { - if (!eval->Context.HasBoundOperand()) { - reportError(eval, content->GetOriginalExpression(), - "$<_0> may only be used inside the body of a binding " - "operation."); + if (!eval->Context.HasBoundOperand(this->Index)) { + std::size_t const count = eval->Context.BoundOperandCount(); + if (count == 0) { + reportError(eval, content->GetOriginalExpression(), + cmStrCat("$<_", this->Index, + "> may only be used inside the body of a binding " + "operation.")); + } else { + reportError( + eval, content->GetOriginalExpression(), + cmStrCat( + "$<_", this->Index, + "> is out of range for the current binding operation, which " + "binds only ", + count, " operand(s) (maximum $<_", count - 1, ">).")); + } return std::string(); } - return eval->Context.GetBoundOperand(); + return eval->Context.GetBoundOperand(this->Index); } -} boundOperandNode; + + std::size_t Index; +}; +static BoundOperandNode const boundOperandNode0{ 0 }; +static BoundOperandNode const boundOperandNode1{ 1 }; static const struct OneNode buildInterfaceNode; @@ -6279,7 +6307,8 @@ cmGeneratorExpressionNode const* cmGeneratorExpressionNode::GetNode( { "PATH_EQUAL", &pathEqualNode }, { "MAKE_C_IDENTIFIER", &makeCIdentifierNode }, { "BOOL", &boolNode }, - { "_0", &boundOperandNode }, + { "_0", &boundOperandNode0 }, + { "_1", &boundOperandNode1 }, { "IF", &ifNode }, { "ANGLE-R", &angle_rNode }, { "COMMA", &commaNode }, diff --git a/Tests/CMakeLib/testGenExBoundOperand.cxx b/Tests/CMakeLib/testGenExBoundOperand.cxx index aa2380e6a1..8f527611d6 100644 --- a/Tests/CMakeLib/testGenExBoundOperand.cxx +++ b/Tests/CMakeLib/testGenExBoundOperand.cxx @@ -2,6 +2,7 @@ file LICENSE.rst or https://cmake.org/licensing for details. */ #include #include +#include #include "cmGenExContext.h" @@ -9,22 +10,57 @@ static bool testContextBinding() { cm::GenEx::Context ctx(nullptr, "Debug"); bool ok = true; - if (ctx.HasBoundOperand()) { + if (ctx.HasBoundOperand() || ctx.BoundOperandCount() != 0) { std::cerr << "binding should start unset\n"; ok = false; } ctx.SetBoundOperand("net"); - if (!ctx.HasBoundOperand() || ctx.GetBoundOperand() != "net") { + if (!ctx.HasBoundOperand() || ctx.BoundOperandCount() != 1 || + ctx.GetBoundOperand() != "net") { std::cerr << "binding did not round-trip\n"; ok = false; } return ok; } +static bool testContextMultipleOperands() +{ + cm::GenEx::Context ctx(nullptr, "Debug"); + bool ok = true; + ctx.SetBoundOperands({ "a", "b" }); + if (ctx.BoundOperandCount() != 2 || !ctx.HasBoundOperand(0) || + !ctx.HasBoundOperand(1) || ctx.GetBoundOperand(0) != "a" || + ctx.GetBoundOperand(1) != "b") { + std::cerr << "two-operand binding did not round-trip\n"; + ok = false; + } + if (ctx.HasBoundOperand(2)) { + std::cerr << "index past the frame should be out of range\n"; + ok = false; + } + // Re-binding replaces the whole frame, which the shadow/restore of nested + // bindings relies on. + ctx.SetBoundOperand("x"); + if (ctx.BoundOperandCount() != 1 || ctx.HasBoundOperand(1) || + ctx.GetBoundOperand(0) != "x") { + std::cerr << "re-binding did not replace the frame\n"; + ok = false; + } + ctx.SetBoundOperands({}); + if (ctx.BoundOperandCount() != 0 || ctx.HasBoundOperand(0)) { + std::cerr << "empty frame should clear the binding\n"; + ok = false; + } + return ok; +} + int testGenExBoundOperand(int /*argc*/, char* /*argv*/[]) { if (!testContextBinding()) { return 1; } + if (!testContextMultipleOperands()) { + return 1; + } return 0; } diff --git a/Tests/RunCMake/GeneratorExpression/BoundOperand1OutsideBinding-result.txt b/Tests/RunCMake/GeneratorExpression/BoundOperand1OutsideBinding-result.txt new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/BoundOperand1OutsideBinding-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/GeneratorExpression/BoundOperand1OutsideBinding-stderr.txt b/Tests/RunCMake/GeneratorExpression/BoundOperand1OutsideBinding-stderr.txt new file mode 100644 index 0000000000..a98c3fabe2 --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/BoundOperand1OutsideBinding-stderr.txt @@ -0,0 +1 @@ +is out of range for the current binding operation diff --git a/Tests/RunCMake/GeneratorExpression/BoundOperand1OutsideBinding.cmake b/Tests/RunCMake/GeneratorExpression/BoundOperand1OutsideBinding.cmake new file mode 100644 index 0000000000..24cf9f76d1 --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/BoundOperand1OutsideBinding.cmake @@ -0,0 +1,4 @@ +# $<_1> requires a binary binding (e.g. SORT COMPARATOR); using it in a unary +# APPLY body, which binds only $<_0>, is an error. +file(GENERATE OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/x.txt" + CONTENT "$Y>") diff --git a/Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake b/Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake index db24f953ce..49ab678fd5 100644 --- a/Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake +++ b/Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake @@ -72,6 +72,7 @@ run_cmake(ListTransformPredicateLinkLibraries) run_cmake(ListFilterPredicateMissingBody) run_cmake(ListFilterPredicateNonBool) run_cmake(BoundOperandOutsideBinding) +run_cmake(BoundOperand1OutsideBinding) function(run_cmake_build test) set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/${test}-build) From c418a3dc856ab4eea275c0a2fc6ecc3b58effbf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Germain?= Date: Fri, 19 Jun 2026 15:57:15 -0700 Subject: [PATCH 2/3] GenEx: factor out reusable SORT comparator and option-parsing helpers Prepare for a genex-driven SORT comparator without changing behavior: let cmList::sort() accept a caller-supplied comparison function, and give the $ COMPARE:/CASE:/ORDER: option parsing a single definition shared with the canned handler. The new feature can then reuse both rather than duplicating the sort plumbing and the option syntax. Issue: #27892 --- Source/cmGeneratorExpressionNode.cxx | 184 ++++++++++++++------------- Source/cmList.cxx | 24 +++- Source/cmList.h | 4 + 3 files changed, 121 insertions(+), 91 deletions(-) diff --git a/Source/cmGeneratorExpressionNode.cxx b/Source/cmGeneratorExpressionNode.cxx index 28c3e82ca4..4ce10d8a93 100644 --- a/Source/cmGeneratorExpressionNode.cxx +++ b/Source/cmGeneratorExpressionNode.cxx @@ -2130,6 +2130,96 @@ std::string EvaluateTransformPredicate( } } +enum class SortOptionResult +{ + NotRecognized, // `arg` is not a SORT option keyword + Parsed, // recognized and applied to `sortConfig` + Error, // recognized but malformed or duplicate (already reported) +}; + +// Parse one $ colon-option (COMPARE:/CASE:/ORDER:) into sortConfig. +SortOptionResult ParseSortOption(std::string const& arg, + cmList::SortConfiguration& sortConfig, + cm::GenEx::Evaluation* eval, + GeneratorExpressionContent const* content) +{ + using SortConfig = cmList::SortConfiguration; + auto const COMPARE = "COMPARE:"_s; + auto const CASE = "CASE:"_s; + auto const ORDER = "ORDER:"_s; + + if (cmHasPrefix(arg, COMPARE)) { + if (sortConfig.Compare != SortConfig::CompareMethod::DEFAULT) { + reportError(eval, content->GetOriginalExpression(), + "sub-command SORT, COMPARE option has been specified " + "multiple times."); + return SortOptionResult::Error; + } + auto option = cm::string_view{ arg.c_str() + COMPARE.length() }; + if (option == "STRING"_s) { + sortConfig.Compare = SortConfig::CompareMethod::STRING; + } else if (option == "FILE_BASENAME"_s) { + sortConfig.Compare = SortConfig::CompareMethod::FILE_BASENAME; + } else if (option == "NATURAL"_s) { + sortConfig.Compare = SortConfig::CompareMethod::NATURAL; + } else { + reportError(eval, content->GetOriginalExpression(), + cmStrCat("sub-command SORT, an invalid COMPARE option has " + "been specified: \"", + option, "\".")); + return SortOptionResult::Error; + } + return SortOptionResult::Parsed; + } + + if (cmHasPrefix(arg, CASE)) { + if (sortConfig.Case != SortConfig::CaseSensitivity::DEFAULT) { + reportError(eval, content->GetOriginalExpression(), + "sub-command SORT, CASE option has been specified multiple " + "times."); + return SortOptionResult::Error; + } + auto option = cm::string_view{ arg.c_str() + CASE.length() }; + if (option == "SENSITIVE"_s) { + sortConfig.Case = SortConfig::CaseSensitivity::SENSITIVE; + } else if (option == "INSENSITIVE"_s) { + sortConfig.Case = SortConfig::CaseSensitivity::INSENSITIVE; + } else { + reportError(eval, content->GetOriginalExpression(), + cmStrCat("sub-command SORT, an invalid CASE option has been " + "specified: \"", + option, "\".")); + return SortOptionResult::Error; + } + return SortOptionResult::Parsed; + } + + if (cmHasPrefix(arg, ORDER)) { + if (sortConfig.Order != SortConfig::OrderMode::DEFAULT) { + reportError(eval, content->GetOriginalExpression(), + "sub-command SORT, ORDER option has been specified multiple " + "times."); + return SortOptionResult::Error; + } + auto option = cm::string_view{ arg.c_str() + ORDER.length() }; + if (option == "ASCENDING"_s) { + sortConfig.Order = SortConfig::OrderMode::ASCENDING; + } else if (option == "DESCENDING"_s) { + sortConfig.Order = SortConfig::OrderMode::DESCENDING; + } else { + reportError( + eval, content->GetOriginalExpression(), + cmStrCat("sub-command SORT, an invalid ORDER option has been " + "specified: \"", + option, "\".")); + return SortOptionResult::Error; + } + return SortOptionResult::Parsed; + } + + return SortOptionResult::NotRecognized; +} + // Parse the optional trailing selector of a $ action // (AT ... / FOR [] / REGEX ) into a // cmList::TransformSelector. Returns nullptr (after reporting via `eval`) on @@ -2706,97 +2796,19 @@ static const struct ListNode : public cmGeneratorExpressionNode false)) { auto list = GetList(args.front()); args.advance(1); - auto const COMPARE = "COMPARE:"_s; - auto const CASE = "CASE:"_s; - auto const ORDER = "ORDER:"_s; - using SortConfig = cmList::SortConfiguration; - SortConfig sortConfig; + cmList::SortConfiguration sortConfig; for (auto const& arg : args) { - if (cmHasPrefix(arg, COMPARE)) { - if (sortConfig.Compare != - SortConfig::CompareMethod::DEFAULT) { - reportError(ev, cnt->GetOriginalExpression(), - "sub-command SORT, COMPARE option has been " - "specified multiple times."); + switch (ParseSortOption(arg, sortConfig, ev, cnt)) { + case SortOptionResult::Parsed: + break; + case SortOptionResult::Error: return std::string{}; - } - auto option = - cm::string_view{ arg.c_str() + COMPARE.length() }; - if (option == "STRING"_s) { - sortConfig.Compare = SortConfig::CompareMethod::STRING; - continue; - } - if (option == "FILE_BASENAME"_s) { - sortConfig.Compare = - SortConfig::CompareMethod::FILE_BASENAME; - continue; - } - if (option == "NATURAL"_s) { - sortConfig.Compare = SortConfig::CompareMethod::NATURAL; - continue; - } - reportError( - ev, cnt->GetOriginalExpression(), - cmStrCat( - "sub-command SORT, an invalid COMPARE option has been " - "specified: \"", - option, "\".")); - return std::string{}; - } - if (cmHasPrefix(arg, CASE)) { - if (sortConfig.Case != - SortConfig::CaseSensitivity::DEFAULT) { + case SortOptionResult::NotRecognized: reportError(ev, cnt->GetOriginalExpression(), - "sub-command SORT, CASE option has been " - "specified multiple times."); + cmStrCat("sub-command SORT, option \"", arg, + "\" is invalid.")); return std::string{}; - } - auto option = cm::string_view{ arg.c_str() + CASE.length() }; - if (option == "SENSITIVE"_s) { - sortConfig.Case = SortConfig::CaseSensitivity::SENSITIVE; - continue; - } - if (option == "INSENSITIVE"_s) { - sortConfig.Case = SortConfig::CaseSensitivity::INSENSITIVE; - continue; - } - reportError( - ev, cnt->GetOriginalExpression(), - cmStrCat( - "sub-command SORT, an invalid CASE option has been " - "specified: \"", - option, "\".")); - return std::string{}; } - if (cmHasPrefix(arg, ORDER)) { - if (sortConfig.Order != SortConfig::OrderMode::DEFAULT) { - reportError(ev, cnt->GetOriginalExpression(), - "sub-command SORT, ORDER option has been " - "specified multiple times."); - return std::string{}; - } - auto option = - cm::string_view{ arg.c_str() + ORDER.length() }; - if (option == "ASCENDING"_s) { - sortConfig.Order = SortConfig::OrderMode::ASCENDING; - continue; - } - if (option == "DESCENDING"_s) { - sortConfig.Order = SortConfig::OrderMode::DESCENDING; - continue; - } - reportError( - ev, cnt->GetOriginalExpression(), - cmStrCat( - "sub-command SORT, an invalid ORDER option has been " - "specified: \"", - option, "\".")); - return std::string{}; - } - reportError(ev, cnt->GetOriginalExpression(), - cmStrCat("sub-command SORT, option \"", arg, - "\" is invalid.")); - return std::string{}; } return list.sort(sortConfig).to_string(); diff --git a/Source/cmList.cxx b/Source/cmList.cxx index a5504090a5..aa5be7a649 100644 --- a/Source/cmList.cxx +++ b/Source/cmList.cxx @@ -382,7 +382,9 @@ cmList& cmList::sort(SortConfiguration cfg) return *this; } -cmList& cmList::sort(SortConfiguration cfg, cmMakefile& makefile) +cmList& cmList::sort( + SortConfiguration cfg, + std::function comparator) { SortConfiguration config{ cfg }; @@ -394,11 +396,10 @@ cmList& cmList::sort(SortConfiguration cfg, cmMakefile& makefile) } try { - ComparatorEvaluator evaluator(config.ComparatorFunction, makefile); StringSorter sorter( - config, [&evaluator](std::string const& a, std::string const& b) { - bool result = evaluator(a, b); - if (result && evaluator(b, a)) { + config, [&comparator](std::string const& a, std::string const& b) { + bool result = comparator(a, b); + if (result && comparator(b, a)) { throw cmList::transform_error( "sub-command SORT, COMPARATOR: function does not induce a strict " "weak ordering. The comparator returned TRUE for both (a, b) and " @@ -414,6 +415,19 @@ cmList& cmList::sort(SortConfiguration cfg, cmMakefile& makefile) return *this; } +cmList& cmList::sort(SortConfiguration cfg, cmMakefile& makefile) +{ + try { + ComparatorEvaluator evaluator(cfg.ComparatorFunction, makefile); + return this->sort( + cfg, [&evaluator](std::string const& a, std::string const& b) { + return evaluator(a, b); + }); + } catch (transform_error& e) { + throw std::invalid_argument(e.what()); + } +} + namespace { using transform_type = std::function; using transform_error = cmList::transform_error; diff --git a/Source/cmList.h b/Source/cmList.h index d8bc4cc912..7ca3076eda 100644 --- a/Source/cmList.h +++ b/Source/cmList.h @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -873,6 +874,9 @@ public: }; cmList& sort(SortConfiguration config = SortConfiguration{}); cmList& sort(SortConfiguration config, cmMakefile& makefile); + cmList& sort( + SortConfiguration config, + std::function comparator); // exception raised on error during transform operations class transform_error : public std::runtime_error From 4d86ef5e861024c06df27eca23756156ee5c1f6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Germain?= Date: Fri, 19 Jun 2026 15:57:22 -0700 Subject: [PATCH 3/3] GenEx: add $ comparator Add a COMPARATOR form to $ that orders the list by a caller-defined rule: a evaluated per comparison with the two elements bound to $<_0> and $<_1>, yielding "1" when the first should sort before the second. This brings the custom ordering of list(SORT ... COMPARATOR) to generate time, so elements can be ordered by target properties or any other generator expression. CASE: and ORDER: still apply, while COMPARE: is rejected because the body defines the ordering. Fixes: #27892 --- Help/manual/cmake-generator-expressions.7.rst | 32 ++++ .../dev/genex-list-sort-comparator.rst | 7 + Source/cmGeneratorExpressionNode.cxx | 99 +++++++++++ Tests/CMakeLib/CMakeLists.txt | 1 + .../CMakeLib/testGenExListSortComparator.cxx | 157 ++++++++++++++++++ ...stSortComparatorCompareConflict-result.txt | 1 + ...stSortComparatorCompareConflict-stderr.txt | 1 + .../ListSortComparatorCompareConflict.cmake | 2 + ...ListSortComparatorInvalidOption-result.txt | 1 + ...ListSortComparatorInvalidOption-stderr.txt | 1 + .../ListSortComparatorInvalidOption.cmake | 4 + .../ListSortComparatorMissingBody-result.txt | 1 + .../ListSortComparatorMissingBody-stderr.txt | 1 + .../ListSortComparatorMissingBody.cmake | 2 + .../ListSortComparatorNonBool-result.txt | 1 + .../ListSortComparatorNonBool-stderr.txt | 1 + .../ListSortComparatorNonBool.cmake | 2 + ...ListSortComparatorNotStrictWeak-result.txt | 1 + ...ListSortComparatorNotStrictWeak-stderr.txt | 1 + .../ListSortComparatorNotStrictWeak.cmake | 2 + ...stSortComparatorTargetProperty-check.cmake | 5 + .../ListSortComparatorTargetProperty.cmake | 10 ++ .../GeneratorExpression/RunCMakeTest.cmake | 6 + 23 files changed, 339 insertions(+) create mode 100644 Help/release/dev/genex-list-sort-comparator.rst create mode 100644 Tests/CMakeLib/testGenExListSortComparator.cxx create mode 100644 Tests/RunCMake/GeneratorExpression/ListSortComparatorCompareConflict-result.txt create mode 100644 Tests/RunCMake/GeneratorExpression/ListSortComparatorCompareConflict-stderr.txt create mode 100644 Tests/RunCMake/GeneratorExpression/ListSortComparatorCompareConflict.cmake create mode 100644 Tests/RunCMake/GeneratorExpression/ListSortComparatorInvalidOption-result.txt create mode 100644 Tests/RunCMake/GeneratorExpression/ListSortComparatorInvalidOption-stderr.txt create mode 100644 Tests/RunCMake/GeneratorExpression/ListSortComparatorInvalidOption.cmake create mode 100644 Tests/RunCMake/GeneratorExpression/ListSortComparatorMissingBody-result.txt create mode 100644 Tests/RunCMake/GeneratorExpression/ListSortComparatorMissingBody-stderr.txt create mode 100644 Tests/RunCMake/GeneratorExpression/ListSortComparatorMissingBody.cmake create mode 100644 Tests/RunCMake/GeneratorExpression/ListSortComparatorNonBool-result.txt create mode 100644 Tests/RunCMake/GeneratorExpression/ListSortComparatorNonBool-stderr.txt create mode 100644 Tests/RunCMake/GeneratorExpression/ListSortComparatorNonBool.cmake create mode 100644 Tests/RunCMake/GeneratorExpression/ListSortComparatorNotStrictWeak-result.txt create mode 100644 Tests/RunCMake/GeneratorExpression/ListSortComparatorNotStrictWeak-stderr.txt create mode 100644 Tests/RunCMake/GeneratorExpression/ListSortComparatorNotStrictWeak.cmake create mode 100644 Tests/RunCMake/GeneratorExpression/ListSortComparatorTargetProperty-check.cmake create mode 100644 Tests/RunCMake/GeneratorExpression/ListSortComparatorTargetProperty.cmake diff --git a/Help/manual/cmake-generator-expressions.7.rst b/Help/manual/cmake-generator-expressions.7.rst index 4506c3761b..68b831e5d7 100644 --- a/Help/manual/cmake-generator-expressions.7.rst +++ b/Help/manual/cmake-generator-expressions.7.rst @@ -1098,6 +1098,24 @@ List Ordering $ + .. versionadded:: 4.5 + + A ``COMPARATOR`` option sorts using a generator-expression ``body`` instead + of a built-in ordering: + + .. code-block:: cmake + + $ + + ``body`` is evaluated once per comparison with the two items being compared + bound to :genex:`$<_0>` and :genex:`$<_1>`. It must evaluate to exactly + ``0`` or ``1``; ``1`` means :genex:`$<_0>` sorts before :genex:`$<_1>`. + ``COMPARATOR`` is incompatible with ``COMPARE:``. ``ORDER:DESCENDING`` + reverses the comparator and ``CASE:INSENSITIVE`` case-folds the values the + body sees, both as in the configure-time :command:`list(SORT)`. The body + must induce a `strict weak ordering + `_. + .. _GenEx Bound Operands: Bound Operands @@ -1117,6 +1135,20 @@ Bound Operands ``$<_0>`` is only valid inside the body of a binding operation. Using it anywhere else is an error. +.. genex:: $<_1> + + .. versionadded:: 4.5 + + The second *bound operand* of a *binding operation* that binds at least two + operands, expanding to the second supplied value. + + For example, :genex:`$` evaluates ``body`` + once per comparison with :genex:`$<_0>` and ``$<_1>`` bound to the two items + being compared. + + ``$<_1>`` is only valid inside the body of a binding operation that binds at + least two operands. Using it anywhere else is an error. + Path Expressions ---------------- diff --git a/Help/release/dev/genex-list-sort-comparator.rst b/Help/release/dev/genex-list-sort-comparator.rst new file mode 100644 index 0000000000..4c3e6bcaf6 --- /dev/null +++ b/Help/release/dev/genex-list-sort-comparator.rst @@ -0,0 +1,7 @@ +genex-list-sort-comparator +-------------------------- + +* The :genex:`LIST` generator expression's ``SORT`` operation gained a + ``COMPARATOR`` option that orders items using an arbitrary generator + expression evaluated once per comparison, with ``$<_0>`` and ``$<_1>`` + referring to the two items being compared. diff --git a/Source/cmGeneratorExpressionNode.cxx b/Source/cmGeneratorExpressionNode.cxx index 4ce10d8a93..473ff7966e 100644 --- a/Source/cmGeneratorExpressionNode.cxx +++ b/Source/cmGeneratorExpressionNode.cxx @@ -2220,6 +2220,88 @@ SortOptionResult ParseSortOption(std::string const& arg, return SortOptionResult::NotRecognized; } +// $: sort with a per-comparison genex body, the +// two elements bound to $<_0> and $<_1>; body must yield "0" or "1". +std::string EvaluateSortComparator(std::vector const& parameters, + std::size_t comparatorIndex, + cm::GenEx::Evaluation* eval, + GeneratorExpressionContent const* content, + cmGeneratorExpressionDAGChecker* dagChecker) +{ + if (comparatorIndex + 1 >= parameters.size()) { + reportError(eval, content->GetOriginalExpression(), + "sub-command SORT, COMPARATOR expects a argument."); + return std::string(); + } + cmGeneratorExpressionEvaluatorVector const& bodyExpr = + content->GetParamChildren()[comparatorIndex + 1]; + + using SortConfig = cmList::SortConfiguration; + SortConfig sortConfig; + sortConfig.Compare = SortConfig::CompareMethod::COMPARATOR; + for (std::size_t i = 2; i < parameters.size(); ++i) { + if (i == comparatorIndex || i == comparatorIndex + 1) { + continue; // COMPARATOR keyword + its (empty) body slot + } + std::string const& arg = parameters[i]; + // COMPARATOR defines the ordering, so reject COMPARE:; CASE:/ORDER: are + // accepted as in list(SORT ... COMPARATOR) (CASE: folds the body + // operands). + if (cmHasPrefix(arg, "COMPARE:"_s)) { + reportError(eval, content->GetOriginalExpression(), + "sub-command SORT, option \"COMPARE\" is incompatible with " + "\"COMPARATOR\"."); + return std::string(); + } + switch (ParseSortOption(arg, sortConfig, eval, content)) { + case SortOptionResult::Parsed: + break; + case SortOptionResult::Error: + return std::string(); + case SortOptionResult::NotRecognized: + reportError( + eval, content->GetOriginalExpression(), + cmStrCat("sub-command SORT, option \"", arg, "\" is invalid.")); + return std::string(); + } + } + + cmList list = GetList(parameters[1]); + if (list.size() < 2) { + return list.to_string(); + } + + // The strict-weak-ordering guard in cmList::sort may call this twice per + // pair, so the body can be evaluated up to twice per comparison. + auto comparator = [&](std::string const& a, std::string const& b) -> bool { + std::string r = + EvaluateBodyWithBoundOperands(bodyExpr, { a, b }, eval, dagChecker); + if (eval->HadError) { + throw cmList::transform_error(std::string{}); // body already reported + } + if (r == "1") { + return true; + } + if (r == "0") { + return false; + } + throw cmList::transform_error( + cmStrCat("sub-command SORT, COMPARATOR body must evaluate to \"0\" or " + "\"1\", but evaluated to \"", + r, "\".")); + }; + + try { + list.sort(sortConfig, comparator); + } catch (std::invalid_argument& e) { + if (!eval->HadError) { + reportError(eval, content->GetOriginalExpression(), e.what()); + } + return std::string(); + } + return list.to_string(); +} + // Parse the optional trailing selector of a $ action // (AT ... / FOR [] / REGEX ) into a // cmList::TransformSelector. Returns nullptr (after reporting via `eval`) on @@ -2363,6 +2445,12 @@ static const struct ListNode : public cmGeneratorExpressionNode return false; } } + // Leave the SORT COMPARATOR unevaluated; a bare COMPARATOR token is + // unambiguous since SORT's other options are colon-style. + if (parameters.size() >= 3 && parameters[0] == "SORT" && + parameters.back() == "COMPARATOR") { + return false; + } // Skip the APPLY (4th parameter) so $<_0> is not evaluated unbound; // selector args (5th+) evaluate normally. return !(parameters.size() == 3 && parameters[0] == "TRANSFORM" && @@ -2482,6 +2570,17 @@ static const struct ListNode : public cmGeneratorExpressionNode .to_string(); } + // SORT COMPARATOR is handled here, not the listCommands SORT lambda, + // because the body needs the DAG checker. + if (parameters.size() >= 3 && parameters[0] == "SORT") { + for (std::size_t i = 2; i < parameters.size(); ++i) { + if (parameters[i] == "COMPARATOR") { + return EvaluateSortComparator(parameters, i, eval, content, + dagChecker); + } + } + } + static std::unordered_map< cm::string_view, std::function +#include + +#include + +#include "cmGeneratorExpression.h" +#include "cmGlobalGenerator.h" +#include "cmLocalGenerator.h" +#include "cmMakefile.h" +#include "cmState.h" +#include "cmStateDirectory.h" +#include "cmStateSnapshot.h" +#include "cmake.h" + +namespace { +struct GenExFixture +{ + cmake CMake{ cmState::Role::Project }; + std::unique_ptr GG; + std::unique_ptr MF; + std::unique_ptr LG; + + GenExFixture() + { + this->GG = cm::make_unique(&this->CMake); + cmStateSnapshot snapshot = this->CMake.GetCurrentSnapshot(); + snapshot.GetDirectory().SetCurrentBinary("."); + snapshot.GetDirectory().SetCurrentSource("."); + this->MF = cm::make_unique(this->GG.get(), snapshot); + this->LG = this->GG->CreateLocalGenerator(this->MF.get()); + } + + std::string Eval(std::string const& expr) + { + return cmGeneratorExpression::Evaluate(expr, this->LG.get(), "Debug"); + } +}; + +bool expectEq(char const* name, std::string const& got, + std::string const& want) +{ + if (got != want) { + std::cerr << name << ": expected '" << want << "', got '" << got << "'\n"; + return false; + } + return true; +} +} + +static bool testSortNumericAscending() +{ + GenExFixture fx; + return expectEq( + "testSortNumericAscending", + fx.Eval("$,$<_1>>>"), "1;2;3"); +} + +static bool testSortByExtension() +{ + GenExFixture fx; + return expectEq("testSortByExtension", + fx.Eval("$>," + "$>>>"), + "a.a;b.m;c.z"); +} + +static bool testSortDescending() +{ + GenExFixture fx; + return expectEq( + "testSortDescending", + fx.Eval( + "$,$<_1>>,ORDER:DESCENDING>"), + "3;2;1"); +} + +static bool testSortNestedBinding() +{ + GenExFixture fx; + // Nested binding: the inner APPLY rebinds $<_0> but the outer $<_1> is + // restored after it, so this reduces to STRLESS(a, b) ascending. + return expectEq( + "testSortNestedBinding", + fx.Eval("$,APPLY,$<_0>>,$<_1>>>"), + "x;y"); +} + +static bool testSortEmpty() +{ + GenExFixture fx; + return expectEq("testSortEmpty", + fx.Eval("$,$<_1>>>"), + ""); +} + +static bool testSortSingle() +{ + GenExFixture fx; + return expectEq("testSortSingle", + fx.Eval("$,$<_1>>>"), + "x"); +} + +static bool testSortEqualElements() +{ + GenExFixture fx; + // Equal elements are FALSE both ways, so the strict-weak-ordering guard must + // not trip and the duplicates are preserved. + return expectEq( + "testSortEqualElements", + fx.Eval("$,$<_1>>>"), "a;b;b"); +} + +static bool testSortCaseInsensitive() +{ + // CASE:INSENSITIVE case-folds the body's operands, so B;a;C orders as a;B;C + // (elements keep their original case). + GenExFixture fx; + return expectEq( + "testSortCaseInsensitive", + fx.Eval( + "$,$<_1>>,CASE:INSENSITIVE>"), + "a;B;C"); +} + +int testGenExListSortComparator(int /*argc*/, char* /*argv*/[]) +{ + if (!testSortNumericAscending()) { + return 1; + } + if (!testSortByExtension()) { + return 1; + } + if (!testSortDescending()) { + return 1; + } + if (!testSortNestedBinding()) { + return 1; + } + if (!testSortEmpty()) { + return 1; + } + if (!testSortSingle()) { + return 1; + } + if (!testSortEqualElements()) { + return 1; + } + if (!testSortCaseInsensitive()) { + return 1; + } + return 0; +} diff --git a/Tests/RunCMake/GeneratorExpression/ListSortComparatorCompareConflict-result.txt b/Tests/RunCMake/GeneratorExpression/ListSortComparatorCompareConflict-result.txt new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/ListSortComparatorCompareConflict-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/GeneratorExpression/ListSortComparatorCompareConflict-stderr.txt b/Tests/RunCMake/GeneratorExpression/ListSortComparatorCompareConflict-stderr.txt new file mode 100644 index 0000000000..2b3012f19e --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/ListSortComparatorCompareConflict-stderr.txt @@ -0,0 +1 @@ +option "COMPARE" is incompatible with "COMPARATOR" diff --git a/Tests/RunCMake/GeneratorExpression/ListSortComparatorCompareConflict.cmake b/Tests/RunCMake/GeneratorExpression/ListSortComparatorCompareConflict.cmake new file mode 100644 index 0000000000..e42b6b34a8 --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/ListSortComparatorCompareConflict.cmake @@ -0,0 +1,2 @@ +file(GENERATE OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/x.txt" + CONTENT "$,$<_1>>,COMPARE:STRING>") diff --git a/Tests/RunCMake/GeneratorExpression/ListSortComparatorInvalidOption-result.txt b/Tests/RunCMake/GeneratorExpression/ListSortComparatorInvalidOption-result.txt new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/ListSortComparatorInvalidOption-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/GeneratorExpression/ListSortComparatorInvalidOption-stderr.txt b/Tests/RunCMake/GeneratorExpression/ListSortComparatorInvalidOption-stderr.txt new file mode 100644 index 0000000000..3dd898f22f --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/ListSortComparatorInvalidOption-stderr.txt @@ -0,0 +1 @@ +option "BOGUS:X" is invalid diff --git a/Tests/RunCMake/GeneratorExpression/ListSortComparatorInvalidOption.cmake b/Tests/RunCMake/GeneratorExpression/ListSortComparatorInvalidOption.cmake new file mode 100644 index 0000000000..d225c85ece --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/ListSortComparatorInvalidOption.cmake @@ -0,0 +1,4 @@ +# An unrecognized trailing option on the COMPARATOR path goes through the +# NotRecognized branch of the shared option parser. +file(GENERATE OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/x.txt" + CONTENT "$,$<_1>>,BOGUS:X>") diff --git a/Tests/RunCMake/GeneratorExpression/ListSortComparatorMissingBody-result.txt b/Tests/RunCMake/GeneratorExpression/ListSortComparatorMissingBody-result.txt new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/ListSortComparatorMissingBody-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/GeneratorExpression/ListSortComparatorMissingBody-stderr.txt b/Tests/RunCMake/GeneratorExpression/ListSortComparatorMissingBody-stderr.txt new file mode 100644 index 0000000000..1d44e821e0 --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/ListSortComparatorMissingBody-stderr.txt @@ -0,0 +1 @@ +sub-command SORT, COMPARATOR expects a argument diff --git a/Tests/RunCMake/GeneratorExpression/ListSortComparatorMissingBody.cmake b/Tests/RunCMake/GeneratorExpression/ListSortComparatorMissingBody.cmake new file mode 100644 index 0000000000..b87353018c --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/ListSortComparatorMissingBody.cmake @@ -0,0 +1,2 @@ +file(GENERATE OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/x.txt" + CONTENT "$") diff --git a/Tests/RunCMake/GeneratorExpression/ListSortComparatorNonBool-result.txt b/Tests/RunCMake/GeneratorExpression/ListSortComparatorNonBool-result.txt new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/ListSortComparatorNonBool-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/GeneratorExpression/ListSortComparatorNonBool-stderr.txt b/Tests/RunCMake/GeneratorExpression/ListSortComparatorNonBool-stderr.txt new file mode 100644 index 0000000000..1598b9e257 --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/ListSortComparatorNonBool-stderr.txt @@ -0,0 +1 @@ +COMPARATOR body must evaluate to "0" or "1" diff --git a/Tests/RunCMake/GeneratorExpression/ListSortComparatorNonBool.cmake b/Tests/RunCMake/GeneratorExpression/ListSortComparatorNonBool.cmake new file mode 100644 index 0000000000..b0335f5b73 --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/ListSortComparatorNonBool.cmake @@ -0,0 +1,2 @@ +file(GENERATE OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/x.txt" + CONTENT "$") diff --git a/Tests/RunCMake/GeneratorExpression/ListSortComparatorNotStrictWeak-result.txt b/Tests/RunCMake/GeneratorExpression/ListSortComparatorNotStrictWeak-result.txt new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/ListSortComparatorNotStrictWeak-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/GeneratorExpression/ListSortComparatorNotStrictWeak-stderr.txt b/Tests/RunCMake/GeneratorExpression/ListSortComparatorNotStrictWeak-stderr.txt new file mode 100644 index 0000000000..498871d9bd --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/ListSortComparatorNotStrictWeak-stderr.txt @@ -0,0 +1 @@ +does not induce a strict weak diff --git a/Tests/RunCMake/GeneratorExpression/ListSortComparatorNotStrictWeak.cmake b/Tests/RunCMake/GeneratorExpression/ListSortComparatorNotStrictWeak.cmake new file mode 100644 index 0000000000..378839a126 --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/ListSortComparatorNotStrictWeak.cmake @@ -0,0 +1,2 @@ +file(GENERATE OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/x.txt" + CONTENT "$") diff --git a/Tests/RunCMake/GeneratorExpression/ListSortComparatorTargetProperty-check.cmake b/Tests/RunCMake/GeneratorExpression/ListSortComparatorTargetProperty-check.cmake new file mode 100644 index 0000000000..a4ee41deaf --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/ListSortComparatorTargetProperty-check.cmake @@ -0,0 +1,5 @@ +file(READ "${RunCMake_TEST_BINARY_DIR}/out.txt" actual) +string(STRIP "${actual}" actual) +if(NOT actual STREQUAL "b;c;a") + set(RunCMake_TEST_FAILED "unexpected output: [${actual}]") +endif() diff --git a/Tests/RunCMake/GeneratorExpression/ListSortComparatorTargetProperty.cmake b/Tests/RunCMake/GeneratorExpression/ListSortComparatorTargetProperty.cmake new file mode 100644 index 0000000000..8b0fa502f9 --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/ListSortComparatorTargetProperty.cmake @@ -0,0 +1,10 @@ +add_custom_target(a) +add_custom_target(b) +add_custom_target(c) +set_property(TARGET a PROPERTY MY_RANK 3) +set_property(TARGET b PROPERTY MY_RANK 1) +set_property(TARGET c PROPERTY MY_RANK 2) + +# Sort the target names by their MY_RANK property, ascending. +file(GENERATE OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/out.txt" + CONTENT "$,MY_RANK>,$,MY_RANK>>>\n") diff --git a/Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake b/Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake index 49ab678fd5..5cc6790858 100644 --- a/Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake +++ b/Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake @@ -73,6 +73,12 @@ run_cmake(ListFilterPredicateMissingBody) run_cmake(ListFilterPredicateNonBool) run_cmake(BoundOperandOutsideBinding) run_cmake(BoundOperand1OutsideBinding) +run_cmake(ListSortComparatorNonBool) +run_cmake(ListSortComparatorNotStrictWeak) +run_cmake(ListSortComparatorCompareConflict) +run_cmake(ListSortComparatorInvalidOption) +run_cmake(ListSortComparatorMissingBody) +run_cmake(ListSortComparatorTargetProperty) function(run_cmake_build test) set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/${test}-build)