[clang] fb65b17 - [NFCI] Refactor how KeywordStatus is calculated

Erich Keane via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 3 06:41:48 PDT 2022


Author: Erich Keane
Date: 2022-08-03T06:41:43-07:00
New Revision: fb65b17932e1163adeda943a3a36f9b482b97f47

URL: https://github.com/llvm/llvm-project/commit/fb65b17932e1163adeda943a3a36f9b482b97f47
DIFF: https://github.com/llvm/llvm-project/commit/fb65b17932e1163adeda943a3a36f9b482b97f47.diff

LOG: [NFCI] Refactor how KeywordStatus is calculated

The getKeywordStatus function is a horrible mess of inter-dependent 'if'
statements that depend significantly on the ORDER of the checks.  This
patch removes the dependency on order by checking each set-flag only
once.

It does this by looping through each of the set bits, and checks each
individual flag for its effect, then combines them at the end.

This might slow down startup performance slightly, as there are only a
few hundred keywords, and a vast majority will only get checked 1x
still.

This patch ALSO removes the KEYWORD_CONCEPTS flag, because it has since
become synonymous with C++20.

Differential Revision: https://reviews.llvm.org/D131007

Added: 
    

Modified: 
    clang/include/clang/Basic/TokenKinds.def
    clang/lib/Basic/IdentifierTable.cpp
    clang/test/Lexer/keywords_test.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/TokenKinds.def b/clang/include/clang/Basic/TokenKinds.def
index 84fc0893c8b59..565e5a37be3d8 100644
--- a/clang/include/clang/Basic/TokenKinds.def
+++ b/clang/include/clang/Basic/TokenKinds.def
@@ -9,8 +9,7 @@
 // This file defines the TokenKind database.  This includes normal tokens like
 // tok::ampamp (corresponding to the && token) as well as keywords for various
 // languages.  Users of this file must optionally #define the TOK, KEYWORD,
-// CXX11_KEYWORD, CONCEPTS_KEYWORD, ALIAS, or PPKEYWORD macros to make use of
-// this file.
+// CXX11_KEYWORD, ALIAS, or PPKEYWORD macros to make use of this file.
 //
 //===----------------------------------------------------------------------===//
 
@@ -29,9 +28,6 @@
 #ifndef CXX20_KEYWORD
 #define CXX20_KEYWORD(X,Y) KEYWORD(X,KEYCXX20|(Y))
 #endif
-#ifndef CONCEPTS_KEYWORD
-#define CONCEPTS_KEYWORD(X) CXX20_KEYWORD(X,KEYCONCEPTS)
-#endif
 #ifndef COROUTINES_KEYWORD
 #define COROUTINES_KEYWORD(X) CXX20_KEYWORD(X,KEYCOROUTINES)
 #endif
@@ -259,8 +255,6 @@ PUNCTUATOR(caretcaret,            "^^")
 //   KEYNOCXX - This is a keyword in every non-C++ dialect.
 //   KEYCXX11 - This is a C++ keyword introduced to C++ in C++11
 //   KEYCXX20 - This is a C++ keyword introduced to C++ in C++20
-//   KEYCONCEPTS - This is a keyword if the C++ extensions for concepts
-//                 are enabled.
 //   KEYMODULES - This is a keyword if the C++ extensions for modules
 //                are enabled.
 //   KEYGNU   - This is a keyword if GNU extensions are enabled
@@ -390,10 +384,6 @@ CXX11_KEYWORD(nullptr               , 0)
 CXX11_KEYWORD(static_assert         , KEYMSCOMPAT)
 CXX11_KEYWORD(thread_local          , 0)
 
-// C++20 keywords
-CONCEPTS_KEYWORD(concept)
-CONCEPTS_KEYWORD(requires)
-
 // C++20 / coroutines TS keywords
 COROUTINES_KEYWORD(co_await)
 COROUTINES_KEYWORD(co_return)
@@ -406,6 +396,9 @@ MODULES_KEYWORD(import)
 // C++20 keywords.
 CXX20_KEYWORD(consteval             , 0)
 CXX20_KEYWORD(constinit             , 0)
+CXX20_KEYWORD(concept               , 0)
+CXX20_KEYWORD(requires              , 0)
+
 // Not a CXX20_KEYWORD because it is disabled by -fno-char8_t.
 KEYWORD(char8_t                     , CHAR8SUPPORT)
 
@@ -935,7 +928,6 @@ ANNOTATION(header_unit)
 #undef TYPE_TRAIT_2
 #undef TYPE_TRAIT_1
 #undef TYPE_TRAIT
-#undef CONCEPTS_KEYWORD
 #undef CXX20_KEYWORD
 #undef CXX11_KEYWORD
 #undef KEYWORD

diff  --git a/clang/lib/Basic/IdentifierTable.cpp b/clang/lib/Basic/IdentifierTable.cpp
index 82cee4aa052dd..4bf9e12af83ca 100644
--- a/clang/lib/Basic/IdentifierTable.cpp
+++ b/clang/lib/Basic/IdentifierTable.cpp
@@ -82,7 +82,7 @@ IdentifierTable::IdentifierTable(const LangOptions &LangOpts,
 // Constants for TokenKinds.def
 namespace {
 
-  enum {
+  enum TokenKey : unsigned {
     KEYC99        = 0x1,
     KEYCXX        = 0x2,
     KEYCXX11      = 0x4,
@@ -99,70 +99,146 @@ namespace {
     WCHARSUPPORT  = 0x2000,
     HALFSUPPORT   = 0x4000,
     CHAR8SUPPORT  = 0x8000,
-    KEYCONCEPTS   = 0x10000,
-    KEYOBJC       = 0x20000,
-    KEYZVECTOR    = 0x40000,
-    KEYCOROUTINES = 0x80000,
-    KEYMODULES    = 0x100000,
-    KEYCXX20      = 0x200000,
-    KEYOPENCLCXX  = 0x400000,
-    KEYMSCOMPAT   = 0x800000,
-    KEYSYCL       = 0x1000000,
-    KEYCUDA       = 0x2000000,
+    KEYOBJC       = 0x10000,
+    KEYZVECTOR    = 0x20000,
+    KEYCOROUTINES = 0x40000,
+    KEYMODULES    = 0x80000,
+    KEYCXX20      = 0x100000,
+    KEYOPENCLCXX  = 0x200000,
+    KEYMSCOMPAT   = 0x400000,
+    KEYSYCL       = 0x800000,
+    KEYCUDA       = 0x1000000,
     KEYMAX        = KEYCUDA, // The maximum key
     KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20,
     KEYALL = (KEYMAX | (KEYMAX-1)) & ~KEYNOMS18 &
              ~KEYNOOPENCL // KEYNOMS18 and KEYNOOPENCL are used to exclude.
   };
 
-  /// How a keyword is treated in the selected standard.
+  /// How a keyword is treated in the selected standard. This enum is ordered
+  /// intentionally so that the value that 'wins' is the most 'permissive'.
   enum KeywordStatus {
+    KS_Unknown,     // Not yet calculated. Used when figuring out the status.
     KS_Disabled,    // Disabled
+    KS_Future,      // Is a keyword in future standard
     KS_Extension,   // Is an extension
     KS_Enabled,     // Enabled
-    KS_Future       // Is a keyword in future standard
   };
 
 } // namespace
 
+// This works on a single TokenKey flag and checks the LangOpts to get the
+// KeywordStatus based exclusively on this flag, so that it can be merged in
+// getKeywordStatus. Most should be enabled/disabled, but some might imply
+// 'future' versions, or extensions. Returns 'unknown' unless this is KNOWN to
+// be disabled, and the calling function makes it 'disabled' if no other flag
+// changes it. This is necessary for the KEYNOCXX and KEYNOOPENCL flags.
+static KeywordStatus getKeywordStatusHelper(const LangOptions &LangOpts,
+                                            TokenKey Flag) {
+  // Flag is a single bit version of TokenKey (that is, not
+  // KEYALL/KEYALLCXX/etc), so we can check with == throughout this function.
+  assert((Flag & ~(Flag - 1)) == Flag && "Multiple bits set?");
+
+  switch (Flag) {
+  case KEYC99:
+    // FIXME: This should have KS_Future logic here, but that can only happen if
+    // getFutureCompatDiagKind ALSO gets updated. This is safe, since C mode is
+    // ALWAYS implied.
+    return LangOpts.C99 ? KS_Enabled : KS_Unknown;
+  case KEYC11:
+    // FIXME: This should have KS_Future logic here, but that can only happen if
+    // getFutureCompatDiagKind ALSO gets updated. This is safe, since C mode is
+    // ALWAYS implied.
+    return LangOpts.C11 ? KS_Enabled : KS_Unknown;
+  case KEYCXX:
+    return LangOpts.CPlusPlus ? KS_Enabled : KS_Unknown;
+  case KEYCXX11:
+    if (LangOpts.CPlusPlus11)
+      return KS_Enabled;
+    return LangOpts.CPlusPlus ? KS_Future : KS_Unknown;
+  case KEYCXX20:
+    if (LangOpts.CPlusPlus20)
+      return KS_Enabled;
+    return LangOpts.CPlusPlus ? KS_Future : KS_Unknown;
+  case KEYGNU:
+    return LangOpts.GNUKeywords ? KS_Extension : KS_Unknown;
+  case KEYMS:
+    return LangOpts.MicrosoftExt ? KS_Extension : KS_Unknown;
+  case BOOLSUPPORT:
+    return LangOpts.Bool ? KS_Enabled : KS_Unknown;
+  case KEYALTIVEC:
+    return LangOpts.AltiVec ? KS_Enabled : KS_Unknown;
+  case KEYBORLAND:
+    return LangOpts.Borland ? KS_Extension : KS_Unknown;
+  case KEYOPENCLC:
+    return LangOpts.OpenCL && !LangOpts.OpenCLCPlusPlus ? KS_Enabled
+                                                        : KS_Unknown;
+  case WCHARSUPPORT:
+    return LangOpts.WChar ? KS_Enabled : KS_Unknown;
+  case HALFSUPPORT:
+    return LangOpts.Half ? KS_Enabled : KS_Unknown;
+  case CHAR8SUPPORT:
+    if (LangOpts.Char8) return KS_Enabled;
+    if (LangOpts.CPlusPlus20) return KS_Unknown;
+    return KS_Future;
+  case KEYOBJC:
+    // We treat bridge casts as objective-C keywords so we can warn on them
+    // in non-arc mode.
+    return LangOpts.ObjC ? KS_Enabled : KS_Unknown;
+  case KEYZVECTOR:
+    return LangOpts.ZVector ? KS_Enabled : KS_Unknown;
+  case KEYCOROUTINES:
+    return LangOpts.Coroutines ? KS_Enabled : KS_Unknown;
+  case KEYMODULES:
+    return LangOpts.ModulesTS ? KS_Enabled : KS_Unknown;
+  case KEYOPENCLCXX:
+    return LangOpts.OpenCLCPlusPlus ? KS_Enabled : KS_Unknown;
+  case KEYMSCOMPAT:
+    return LangOpts.MSVCCompat ? KS_Enabled : KS_Unknown;
+  case KEYSYCL:
+    return LangOpts.isSYCL() ? KS_Enabled : KS_Unknown;
+  case KEYCUDA:
+    return LangOpts.CUDA ? KS_Enabled : KS_Unknown;
+  case KEYNOCXX:
+    // This is enabled in all non-C++ modes, but might be enabled for other
+    // reasons as well.
+    return LangOpts.CPlusPlus ? KS_Unknown : KS_Enabled;
+  case KEYNOOPENCL:
+    // The disable behavior for this is handled in getKeywordStatus.
+    return KS_Unknown;
+  case KEYNOMS18:
+    // The disable behavior for this is handled in getKeywordStatus.
+    return KS_Unknown;
+  default:
+    llvm_unreachable("Unknown KeywordStatus flag");
+  }
+}
+
 /// Translates flags as specified in TokenKinds.def into keyword status
 /// in the given language standard.
 static KeywordStatus getKeywordStatus(const LangOptions &LangOpts,
                                       unsigned Flags) {
+  // KEYALL means always enabled, so special case this one.
   if (Flags == KEYALL) return KS_Enabled;
-  if (LangOpts.CPlusPlus && (Flags & KEYCXX)) return KS_Enabled;
-  if (LangOpts.CPlusPlus11 && (Flags & KEYCXX11)) return KS_Enabled;
-  if (LangOpts.CPlusPlus20 && (Flags & KEYCXX20)) return KS_Enabled;
-  if (LangOpts.C99 && (Flags & KEYC99)) return KS_Enabled;
-  if (LangOpts.GNUKeywords && (Flags & KEYGNU)) return KS_Extension;
-  if (LangOpts.MicrosoftExt && (Flags & KEYMS)) return KS_Extension;
-  if (LangOpts.MSVCCompat && (Flags & KEYMSCOMPAT)) return KS_Enabled;
-  if (LangOpts.Borland && (Flags & KEYBORLAND)) return KS_Extension;
-  if (LangOpts.Bool && (Flags & BOOLSUPPORT)) return KS_Enabled;
-  if (LangOpts.Half && (Flags & HALFSUPPORT)) return KS_Enabled;
-  if (LangOpts.WChar && (Flags & WCHARSUPPORT)) return KS_Enabled;
-  if (LangOpts.Char8 && (Flags & CHAR8SUPPORT)) return KS_Enabled;
-  if (LangOpts.AltiVec && (Flags & KEYALTIVEC)) return KS_Enabled;
-  if (LangOpts.ZVector && (Flags & KEYZVECTOR)) return KS_Enabled;
-  if (LangOpts.OpenCL && !LangOpts.OpenCLCPlusPlus && (Flags & KEYOPENCLC))
-    return KS_Enabled;
-  if (LangOpts.OpenCLCPlusPlus && (Flags & KEYOPENCLCXX)) return KS_Enabled;
-  if (!LangOpts.CPlusPlus && (Flags & KEYNOCXX)) return KS_Enabled;
-  if (LangOpts.C11 && (Flags & KEYC11)) return KS_Enabled;
-  // We treat bridge casts as objective-C keywords so we can warn on them
-  // in non-arc mode.
-  if (LangOpts.ObjC && (Flags & KEYOBJC)) return KS_Enabled;
-  if (LangOpts.CPlusPlus20 && (Flags & KEYCONCEPTS)) return KS_Enabled;
-  if (LangOpts.Coroutines && (Flags & KEYCOROUTINES)) return KS_Enabled;
-  if (LangOpts.ModulesTS && (Flags & KEYMODULES)) return KS_Enabled;
-  if (LangOpts.CPlusPlus && (Flags & KEYALLCXX)) return KS_Future;
-  if (LangOpts.CPlusPlus && !LangOpts.CPlusPlus20 && (Flags & CHAR8SUPPORT))
-    return KS_Future;
-  if (LangOpts.isSYCL() && (Flags & KEYSYCL))
-    return KS_Enabled;
-  if (LangOpts.CUDA && (Flags & KEYCUDA))
-    return KS_Enabled;
-  return KS_Disabled;
+  // These are tests that need to 'always win', as they are special in that they
+  // disable based on certain conditions.
+  if (LangOpts.OpenCL && (Flags & KEYNOOPENCL)) return KS_Disabled;
+  if (LangOpts.MSVCCompat && (Flags & KEYNOMS18) &&
+      !LangOpts.isCompatibleWithMSVC(LangOptions::MSVC2015))
+    return KS_Disabled;
+
+  KeywordStatus CurStatus = KS_Unknown;
+
+  while (Flags != 0) {
+    unsigned CurFlag = Flags & ~(Flags - 1);
+    Flags = Flags & ~CurFlag;
+    CurStatus = std::max(
+        CurStatus,
+        getKeywordStatusHelper(LangOpts, static_cast<TokenKey>(CurFlag)));
+  }
+
+  if (CurStatus == KS_Unknown)
+    return KS_Disabled;
+  return CurStatus;
 }
 
 /// AddKeyword - This method is used to associate a token ID with specific
@@ -173,15 +249,6 @@ static void AddKeyword(StringRef Keyword,
                        const LangOptions &LangOpts, IdentifierTable &Table) {
   KeywordStatus AddResult = getKeywordStatus(LangOpts, Flags);
 
-  // Don't add this keyword under MSVCCompat.
-  if (LangOpts.MSVCCompat && (Flags & KEYNOMS18) &&
-      !LangOpts.isCompatibleWithMSVC(LangOptions::MSVC2015))
-    return;
-
-  // Don't add this keyword under OpenCL.
-  if (LangOpts.OpenCL && (Flags & KEYNOOPENCL))
-    return;
-
   // Don't add this keyword if disabled in this language.
   if (AddResult == KS_Disabled) return;
 

diff  --git a/clang/test/Lexer/keywords_test.cpp b/clang/test/Lexer/keywords_test.cpp
index 952d7041fdc27..0fb9a6a086d3f 100644
--- a/clang/test/Lexer/keywords_test.cpp
+++ b/clang/test/Lexer/keywords_test.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -std=c++03 -fsyntax-only %s
 // RUN: %clang_cc1 -std=c++11 -DCXX11 -fsyntax-only %s
-// RUN: %clang_cc1 -std=c++2a -DCXX11 -DCXX2A -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++20 -DCXX11 -DCXX20 -fsyntax-only %s
 // RUN: %clang_cc1 -std=c++03 -fdeclspec -DDECLSPEC -fsyntax-only %s
 // RUN: %clang_cc1 -std=c++03 -fms-extensions -DDECLSPEC -fsyntax-only %s
 // RUN: %clang_cc1 -std=c++03 -fborland-extensions -DDECLSPEC -fsyntax-only %s
@@ -19,10 +19,10 @@
 #define NOT_KEYWORD(NAME) _Static_assert(__is_identifier(NAME), #NAME)
 #define IS_TYPE(NAME) void is_##NAME##_type() { int f(NAME); }
 
-#if defined(CXX2A)
-#define CONCEPTS_KEYWORD(NAME)  IS_KEYWORD(NAME)
+#if defined(CXX20)
+#define CXX20_KEYWORD(NAME)  IS_KEYWORD(NAME)
 #else
-#define CONCEPTS_KEYWORD(NAME)  NOT_KEYWORD(NAME)
+#define CXX20_KEYWORD(NAME)  NOT_KEYWORD(NAME)
 #endif
 
 #ifdef DECLSPEC
@@ -59,8 +59,8 @@ IS_KEYWORD(static_assert);
 CXX11_KEYWORD(thread_local);
 
 // Concepts keywords
-CONCEPTS_KEYWORD(concept);
-CONCEPTS_KEYWORD(requires);
+CXX20_KEYWORD(concept);
+CXX20_KEYWORD(requires);
 
 // __declspec extension
 DECLSPEC_KEYWORD(__declspec);


        


More information about the cfe-commits mailing list