Address review comments

Author: kparzysz

llvm/include/llvm/Frontend/Directive/Spelling.h

@@ -1,4 +1,4 @@
-//===-- Spelling.h ------------------------------------------------ C++ -*-===//
+//===-------------------------------------------------------------- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -12,6 +12,7 @@
 #include "llvm/ADT/iterator_range.h"
 
 #include <limits>
+#include <tuple>
 
 namespace llvm::directive {
 
@@ -21,13 +22,11 @@ struct VersionRange {
   // in the maximum range.
   int Min = 0;
   int Max = MaxValue;
-};
 
-inline bool operator<(const VersionRange &A, const VersionRange &B) {
-  if (A.Min != B.Min)
-    return A.Min < B.Min;
-  return A.Max < B.Max;
-}
+  bool operator<(const VersionRange &R) const {
+    return std::tie(Min, Max) < std::tie(R.Min, R.Max);
+  }
+};
 
 struct Spelling {
   StringRef Name;

llvm/include/llvm/TableGen/DirectiveEmitter.h

@@ -116,7 +116,7 @@ class Versioned {
 
 class Spelling : public Versioned {
 public:
-  using Value = llvm::directive::Spelling;
+  using Value = directive::Spelling;
 
   Spelling(const Record *Def) : Def(Def) {}
 

llvm/lib/Frontend/Directive/Spelling.cpp

@@ -1,4 +1,4 @@
-//===-- Spelling.cpp ---------------------------------------------- C++ -*-===//
+//===-------------------------------------------------------------- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -13,13 +13,14 @@
 
 #include <cassert>
 
-static bool Contains(llvm::directive::VersionRange V, int P) {
+using namespace llvm;
+
+static bool Contains(directive::VersionRange V, int P) {
   return V.Min <= P && P <= V.Max;
 }
 
 llvm::StringRef llvm::directive::FindName(
-    llvm::iterator_range<const llvm::directive::Spelling *> Range,
-    unsigned Version) {
+    llvm::iterator_range<const directive::Spelling *> Range, unsigned Version) {
   assert(llvm::isInt<8 * sizeof(int)>(Version) && "Version value out of range");
 
   int V = Version;

llvm/test/TableGen/directive1.td

@@ -355,7 +355,7 @@ def TDL_DirA : Directive<[Spelling<"dira">]> {
 // IMPL-NEXT:      case TDLC_clauseb:
 // IMPL-NEXT:        return "clauseb";
 // IMPL-NEXT:      case TDLC_clausec: {
-// IMPL-NEXT:        static const llvm::directive::Spelling TDLC_clausec_spellings[] = {
+// IMPL-NEXT:        static constexpr llvm::directive::Spelling TDLC_clausec_spellings[] = {
 // IMPL-NEXT:            {"clausec", {0, 2147483647}},
 // IMPL-NEXT:            {"ccccccc", {0, 2147483647}},
 // IMPL-NEXT:        };

llvm/utils/TableGen/Basic/DirectiveEmitter.cpp

@@ -387,18 +387,6 @@ static std::vector<Spelling::Value>
 orderSpellings(ArrayRef<Spelling::Value> Spellings) {
   std::vector<Spelling::Value> List(Spellings.begin(), Spellings.end());
 
-  // There are two intertwined orderings: (1) the order between spellings
-  // (used here), and (2) the order between a spelling and a version (used
-  // at runtime).
-  // Define order (2) as such that the first A that is not less than V
-  // will be the selected spelling given V. Specifically,
-  //   V <(2) A  <=>  V < A.Min
-  //   A <(2) V  <=>  A.Max < V
-  //
-  // The orders have to be compatible, i.e.
-  //   A <(2) V and !(V <(2) B)  =>  A <(1) B, and
-  //   !(A <(2) v) and V <(2) B  =>  A <(1) B
-  // In other words, the transitive closure of (2) must contain (1).
   llvm::stable_sort(List,
                     [](const Spelling::Value &A, const Spelling::Value &B) {
                       return A.Versions < B.Versions;
@@ -421,16 +409,16 @@ static void generateGetName(ArrayRef<const Record *> Records, raw_ostream &OS,
     BaseRecord Rec(R);
     std::string Ident = getIdentifierName(R, Prefix);
     OS << "    case " << Ident << ":";
-    auto Spellings(orderSpellings(Rec.getSpellings()));
+    std::vector<Spelling::Value> Spellings(orderSpellings(Rec.getSpellings()));
     assert(Spellings.size() != 0 && "No spellings for this item");
     if (Spellings.size() == 1) {
       OS << "\n";
       OS << "      return \"" << Spellings.front().Name << "\";\n";
     } else {
       OS << " {\n";
       std::string SpellingsName = Ident + "_spellings";
-      OS << "      static const llvm::directive::Spelling " << SpellingsName
-         << "[] = {\n";
+      OS << "      static constexpr llvm::directive::Spelling "
+         << SpellingsName << "[] = {\n";
       for (auto &S : Spellings) {
         OS << "          {\"" << S.Name << "\", {" << S.Versions.Min << ", "
            << S.Versions.Max << "}},\n";
@@ -484,11 +472,10 @@ static void generateGetKind(ArrayRef<const Record *> Records, raw_ostream &OS,
 
     for (auto &[Name, Versions] : Rec.getSpellings()) {
       OS << "    .Case(\"" << Name << "\", {" << Ident << ", ";
-      if (Versions.Min == All.Min && Versions.Max == All.Max) {
+      if (Versions.Min == All.Min && Versions.Max == All.Max)
         OS << "All})\n";
-      } else {
+      else
         OS << "{" << Versions.Min << ", " << Versions.Max << "}})\n";
-      }
     }
   }
   OS << "    .Default({" << DefaultName << ", All});\n";