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 $<LIST:SORT> 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
2026-09-25 04:09:36 +03:00 · 2026-06-30 08:19:41 -07:00
parent e335872625
commit c418a3dc85
3 changed files with 121 additions and 91 deletions
@@ -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 $<LIST:SORT> 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 $<LIST:TRANSFORM,...> action
 // (AT <i>... / FOR <start> <stop> [<step>] / REGEX <re>) 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();
@@ -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<bool(std::string const&, std::string const&)> 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<std::string(std::string const&)>;
 using transform_error = cmList::transform_error;
@@ -7,6 +7,7 @@

 #include <algorithm>
 #include <cstddef>
+#include <functional>
 #include <initializer_list>
 #include <iterator>
 #include <memory>
@@ -873,6 +874,9 @@ public:
  };
  cmList& sort(SortConfiguration config = SortConfiguration{});
  cmList& sort(SortConfiguration config, cmMakefile& makefile);
+  cmList& sort(
+    SortConfiguration config,
+    std::function<bool(std::string const&, std::string const&)> comparator);

  // exception raised on error during transform operations
  class transform_error : public std::runtime_error