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] 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