[clang][NFC] Use "notable" for "interesting" identifiers in `IdentifierInfo` #81542

Endilll · 2024-02-12T22:00:18Z

This patch expands notion of "interesting" in IdentifierInto it to also cover ObjC keywords and builtins, which matches notion of "interesting" in serialization layer. What was previously "interesting" in IdentifierInto is now called "notable".

Beyond clearing confusion between serialization and the rest of the compiler, it also resolved a naming problem: ObjC keywords, notable identifiers, and builtin IDs are all stored in the same bit-field. Now we can use "interesting" to name it and its corresponding type, instead of ObjCKeywordOrInterestingOrBuiltin abomination.

… `IdentifierInfo` This patch expands notion of "interesting" in `IdentifierInto` it to also cover ObjC keywords and builtins, which matches noting of "interesting" in serialization layer. What was previously "interesting" in `IdentifierInto` is now called "notable". Beyond clearing confusion between serialization and the rest of the compiler, it also resolved a naming problem: ObjC keywords, notable identifiers, and builtin IDs are all stored in the same bit-field. Now we can use "interesting" to name it and its corresponding type, instead of `ObjCKeywordOrInterestingOrBuiltin` abomination.

llvmbot · 2024-02-12T22:00:47Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: Vlad Serebrennikov (Endilll)

Changes

This patch expands notion of "interesting" in IdentifierInto it to also cover ObjC keywords and builtins, which matches noting of "interesting" in serialization layer. What was previously "interesting" in IdentifierInto is now called "notable".

Beyond clearing confusion between serialization and the rest of the compiler, it also resolved a naming problem: ObjC keywords, notable identifiers, and builtin IDs are all stored in the same bit-field. Now we can use "interesting" to name it and its corresponding type, instead of ObjCKeywordOrInterestingOrBuiltin abomination.

Full diff: https://github.com/llvm/llvm-project/pull/81542.diff

7 Files Affected:

(modified) clang/include/clang/Basic/IdentifierTable.h (+48-56)
(modified) clang/include/clang/Basic/TokenKinds.def (+11-11)
(modified) clang/include/clang/Basic/TokenKinds.h (+4-4)
(modified) clang/lib/Basic/IdentifierTable.cpp (+9-9)
(modified) clang/lib/Sema/SemaDecl.cpp (+7-7)
(modified) clang/lib/Serialization/ASTReader.cpp (+1-2)
(modified) clang/lib/Serialization/ASTWriter.cpp (+2-2)

diff --git a/clang/include/clang/Basic/IdentifierTable.h b/clang/include/clang/Basic/IdentifierTable.h
index fa8969eb73ddbf..a091639bfa2542 100644
--- a/clang/include/clang/Basic/IdentifierTable.h
+++ b/clang/include/clang/Basic/IdentifierTable.h
@@ -84,28 +84,28 @@ using IdentifierLocPair = std::pair<IdentifierInfo *, SourceLocation>;
 /// of a pointer to one of these classes.
 enum { IdentifierInfoAlignment = 8 };
 
-static constexpr int ObjCOrBuiltinIDBits = 16;
+static constexpr int InterestingIdentifierBits = 16;
 
-/// The "layout" of ObjCOrBuiltinID is:
+/// The "layout" of InterestingIdentifier is:
 ///  - ObjCKeywordKind enumerators
-///  - InterestingIdentifierKind enumerators
+///  - NotableIdentifierKind enumerators
 ///  - Builtin::ID enumerators
-///  - NonSpecialIdentifier
-enum class ObjCKeywordOrInterestingOrBuiltin {
+///  - NotInterestingIdentifier
+enum class InterestingIdentifier {
 #define OBJC_AT_KEYWORD(X) objc_##X,
 #include "clang/Basic/TokenKinds.def"
   NUM_OBJC_KEYWORDS,
 
-#define INTERESTING_IDENTIFIER(X) X,
+#define NOTABLE_IDENTIFIER(X) X,
 #include "clang/Basic/TokenKinds.def"
-  NUM_OBJC_KEYWORDS_AND_INTERESTING_IDENTIFIERS,
+  NUM_OBJC_KEYWORDS_AND_NOTABLE_IDENTIFIERS,
 
   NotBuiltin,
 #define BUILTIN(ID, TYPE, ATTRS) BI##ID,
 #include "clang/Basic/Builtins.inc"
   FirstTSBuiltin,
 
-  NonSpecialIdentifier = 65534
+  NotInterestingIdentifier = 65534
 };
 
 /// One of these records is kept for each identifier that
@@ -121,8 +121,8 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
   LLVM_PREFERRED_TYPE(tok::TokenKind)
   unsigned TokenID : 9;
 
-  LLVM_PREFERRED_TYPE(ObjCKeywordOrInterestingOrBuiltin)
-  unsigned ObjCOrBuiltinID : ObjCOrBuiltinIDBits;
+  LLVM_PREFERRED_TYPE(InterestingIdentifier)
+  unsigned InterestingIdentifierID : InterestingIdentifierBits;
 
   // True if there is a #define for this.
   LLVM_PREFERRED_TYPE(bool)
@@ -205,8 +205,8 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
 
   IdentifierInfo()
       : TokenID(tok::identifier),
-        ObjCOrBuiltinID(llvm::to_underlying(
-            ObjCKeywordOrInterestingOrBuiltin::NonSpecialIdentifier)),
+        InterestingIdentifierID(llvm::to_underlying(
+            InterestingIdentifier::NotInterestingIdentifier)),
         HasMacro(false), HadMacro(false), IsExtension(false),
         IsFutureCompatKeyword(false), IsPoisoned(false),
         IsCPPOperatorKeyword(false), NeedsHandleIdentifier(false),
@@ -341,71 +341,63 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
   ///
   /// For example, 'class' will return tok::objc_class if ObjC is enabled.
   tok::ObjCKeywordKind getObjCKeywordID() const {
-    assert(0 == llvm::to_underlying(
-                    ObjCKeywordOrInterestingOrBuiltin::objc_not_keyword));
-    auto Value =
-        static_cast<ObjCKeywordOrInterestingOrBuiltin>(ObjCOrBuiltinID);
-    if (Value < ObjCKeywordOrInterestingOrBuiltin::NUM_OBJC_KEYWORDS)
-      return static_cast<tok::ObjCKeywordKind>(ObjCOrBuiltinID);
+    assert(0 == llvm::to_underlying(InterestingIdentifier::objc_not_keyword));
+    auto Value = static_cast<InterestingIdentifier>(InterestingIdentifierID);
+    if (Value < InterestingIdentifier::NUM_OBJC_KEYWORDS)
+      return static_cast<tok::ObjCKeywordKind>(InterestingIdentifierID);
     return tok::objc_not_keyword;
   }
   void setObjCKeywordID(tok::ObjCKeywordKind ID) {
-    assert(0 == llvm::to_underlying(
-                    ObjCKeywordOrInterestingOrBuiltin::objc_not_keyword));
-    ObjCOrBuiltinID = ID;
+    assert(0 == llvm::to_underlying(InterestingIdentifier::objc_not_keyword));
+    InterestingIdentifierID = ID;
     assert(getObjCKeywordID() == ID && "ID too large for field!");
   }
 
   /// Return a value indicating whether this is a builtin function.
   unsigned getBuiltinID() const {
-    auto Value =
-        static_cast<ObjCKeywordOrInterestingOrBuiltin>(ObjCOrBuiltinID);
-    if (Value > ObjCKeywordOrInterestingOrBuiltin::
-                    NUM_OBJC_KEYWORDS_AND_INTERESTING_IDENTIFIERS &&
-        Value != ObjCKeywordOrInterestingOrBuiltin::NonSpecialIdentifier) {
+    auto Value = static_cast<InterestingIdentifier>(InterestingIdentifierID);
+    if (Value >
+            InterestingIdentifier::NUM_OBJC_KEYWORDS_AND_NOTABLE_IDENTIFIERS &&
+        Value != InterestingIdentifier::NotInterestingIdentifier) {
       auto FirstBuiltin =
-          llvm::to_underlying(ObjCKeywordOrInterestingOrBuiltin::NotBuiltin);
-      return static_cast<Builtin::ID>(ObjCOrBuiltinID - FirstBuiltin);
+          llvm::to_underlying(InterestingIdentifier::NotBuiltin);
+      return static_cast<Builtin::ID>(InterestingIdentifierID - FirstBuiltin);
     }
     return Builtin::ID::NotBuiltin;
   }
   void setBuiltinID(unsigned ID) {
     assert(ID != Builtin::ID::NotBuiltin);
-    auto FirstBuiltin =
-        llvm::to_underlying(ObjCKeywordOrInterestingOrBuiltin::NotBuiltin);
-    ObjCOrBuiltinID = ID + FirstBuiltin;
+    auto FirstBuiltin = llvm::to_underlying(InterestingIdentifier::NotBuiltin);
+    InterestingIdentifierID = ID + FirstBuiltin;
     assert(getBuiltinID() == ID && "ID too large for field!");
   }
   void clearBuiltinID() {
-    ObjCOrBuiltinID = llvm::to_underlying(
-        ObjCKeywordOrInterestingOrBuiltin::NonSpecialIdentifier);
-  }
-
-  tok::InterestingIdentifierKind getInterestingIdentifierID() const {
-    auto Value =
-        static_cast<ObjCKeywordOrInterestingOrBuiltin>(ObjCOrBuiltinID);
-    if (Value > ObjCKeywordOrInterestingOrBuiltin::NUM_OBJC_KEYWORDS &&
-        Value < ObjCKeywordOrInterestingOrBuiltin::
-                    NUM_OBJC_KEYWORDS_AND_INTERESTING_IDENTIFIERS) {
-      auto FirstInterestingIdentifier =
-          1 + llvm::to_underlying(
-                  ObjCKeywordOrInterestingOrBuiltin::NUM_OBJC_KEYWORDS);
-      return static_cast<tok::InterestingIdentifierKind>(
-          ObjCOrBuiltinID - FirstInterestingIdentifier);
+    InterestingIdentifierID =
+        llvm::to_underlying(InterestingIdentifier::NotInterestingIdentifier);
+  }
+
+  tok::NotableIdentifierKind getNotableIdentifierID() const {
+    auto Value = static_cast<InterestingIdentifier>(InterestingIdentifierID);
+    if (Value > InterestingIdentifier::NUM_OBJC_KEYWORDS &&
+        Value <
+            InterestingIdentifier::NUM_OBJC_KEYWORDS_AND_NOTABLE_IDENTIFIERS) {
+      auto FirstNotableIdentifier =
+          1 + llvm::to_underlying(InterestingIdentifier::NUM_OBJC_KEYWORDS);
+      return static_cast<tok::NotableIdentifierKind>(InterestingIdentifierID -
+                                                     FirstNotableIdentifier);
     }
-    return tok::not_interesting;
+    return tok::not_notable;
   }
-  void setInterestingIdentifierID(unsigned ID) {
-    assert(ID != tok::not_interesting);
-    auto FirstInterestingIdentifier =
-        1 + llvm::to_underlying(
-                ObjCKeywordOrInterestingOrBuiltin::NUM_OBJC_KEYWORDS);
-    ObjCOrBuiltinID = ID + FirstInterestingIdentifier;
-    assert(getInterestingIdentifierID() == ID && "ID too large for field!");
+  void setNotableIdentifierID(unsigned ID) {
+    assert(ID != tok::not_notable);
+    auto FirstNotableIdentifier =
+        1 + llvm::to_underlying(InterestingIdentifier::NUM_OBJC_KEYWORDS);
+    InterestingIdentifierID = ID + FirstNotableIdentifier;
+    assert(getNotableIdentifierID() == ID && "ID too large for field!");
   }
 
-  unsigned getObjCOrBuiltinID() const { return ObjCOrBuiltinID; }
-  void setObjCOrBuiltinID(unsigned ID) { ObjCOrBuiltinID = ID; }
+  unsigned getObjCOrBuiltinID() const { return InterestingIdentifierID; }
+  void setObjCOrBuiltinID(unsigned ID) { InterestingIdentifierID = ID; }
 
   /// get/setExtension - Initialize information about whether or not this
   /// language token is an extension.  This controls extension warnings, and is
diff --git a/clang/include/clang/Basic/TokenKinds.def b/clang/include/clang/Basic/TokenKinds.def
index 23817cde7a9354..2046ab9dc0198c 100644
--- a/clang/include/clang/Basic/TokenKinds.def
+++ b/clang/include/clang/Basic/TokenKinds.def
@@ -85,8 +85,8 @@
 #ifndef PRAGMA_ANNOTATION
 #define PRAGMA_ANNOTATION(X) ANNOTATION(X)
 #endif
-#ifndef INTERESTING_IDENTIFIER
-#define INTERESTING_IDENTIFIER(X)
+#ifndef NOTABLE_IDENTIFIER
+#define NOTABLE_IDENTIFIER(X)
 #endif
 
 //===----------------------------------------------------------------------===//
@@ -808,15 +808,15 @@ OBJC_AT_KEYWORD(import)
 OBJC_AT_KEYWORD(available)
 
 //===----------------------------------------------------------------------===//
-// Interesting identifiers.
+// Notable identifiers.
 //===----------------------------------------------------------------------===//
-INTERESTING_IDENTIFIER(not_interesting)
-INTERESTING_IDENTIFIER(FILE)
-INTERESTING_IDENTIFIER(jmp_buf)
-INTERESTING_IDENTIFIER(sigjmp_buf)
-INTERESTING_IDENTIFIER(ucontext_t)
-INTERESTING_IDENTIFIER(float_t)
-INTERESTING_IDENTIFIER(double_t)
+NOTABLE_IDENTIFIER(not_notable)
+NOTABLE_IDENTIFIER(FILE)
+NOTABLE_IDENTIFIER(jmp_buf)
+NOTABLE_IDENTIFIER(sigjmp_buf)
+NOTABLE_IDENTIFIER(ucontext_t)
+NOTABLE_IDENTIFIER(float_t)
+NOTABLE_IDENTIFIER(double_t)
 
 // TODO: What to do about context-sensitive keywords like:
 //       bycopy/byref/in/inout/oneway/out?
@@ -1011,4 +1011,4 @@ ANNOTATION(repl_input_end)
 #undef TOK
 #undef C99_KEYWORD
 #undef C23_KEYWORD
-#undef INTERESTING_IDENTIFIER
+#undef NOTABLE_IDENTIFIER
diff --git a/clang/include/clang/Basic/TokenKinds.h b/clang/include/clang/Basic/TokenKinds.h
index 7529b922619ada..e5183a27d2bc5f 100644
--- a/clang/include/clang/Basic/TokenKinds.h
+++ b/clang/include/clang/Basic/TokenKinds.h
@@ -44,12 +44,12 @@ enum ObjCKeywordKind {
   NUM_OBJC_KEYWORDS
 };
 
-/// Provides a namespace for interesting identifers such as float_t and
+/// Provides a namespace for notable identifers such as float_t and
 /// double_t.
-enum InterestingIdentifierKind {
-#define INTERESTING_IDENTIFIER(X) X,
+enum NotableIdentifierKind {
+#define NOTABLE_IDENTIFIER(X) X,
 #include "clang/Basic/TokenKinds.def"
-  NUM_INTERESTING_IDENTIFIERS
+  NUM_NOTABLE_IDENTIFIERS
 };
 
 /// Defines the possible values of an on-off-switch (C99 6.10.6p2).
diff --git a/clang/lib/Basic/IdentifierTable.cpp b/clang/lib/Basic/IdentifierTable.cpp
index d0d8316385b452..a9b07aca65c052 100644
--- a/clang/lib/Basic/IdentifierTable.cpp
+++ b/clang/lib/Basic/IdentifierTable.cpp
@@ -36,7 +36,7 @@ using namespace clang;
 // A check to make sure the ObjCOrBuiltinID has sufficient room to store the
 // largest possible target/aux-target combination. If we exceed this, we likely
 // need to just change the ObjCOrBuiltinIDBits value in IdentifierTable.h.
-static_assert(2 * LargestBuiltinID < (2 << (ObjCOrBuiltinIDBits - 1)),
+static_assert(2 * LargestBuiltinID < (2 << (InterestingIdentifierBits - 1)),
               "Insufficient ObjCOrBuiltinID Bits");
 
 //===----------------------------------------------------------------------===//
@@ -280,13 +280,13 @@ static void AddObjCKeyword(StringRef Name,
   Table.get(Name).setObjCKeywordID(ObjCID);
 }
 
-static void AddInterestingIdentifier(StringRef Name,
-                                     tok::InterestingIdentifierKind BTID,
-                                     IdentifierTable &Table) {
-  // Don't add 'not_interesting' identifier.
-  if (BTID != tok::not_interesting) {
+static void AddNotableIdentifier(StringRef Name,
+                                 tok::NotableIdentifierKind BTID,
+                                 IdentifierTable &Table) {
+  // Don't add 'not_notable' identifier.
+  if (BTID != tok::not_notable) {
     IdentifierInfo &Info = Table.get(Name, tok::identifier);
-    Info.setInterestingIdentifierID(BTID);
+    Info.setNotableIdentifierID(BTID);
   }
 }
 
@@ -306,8 +306,8 @@ void IdentifierTable::AddKeywords(const LangOptions &LangOpts) {
 #define OBJC_AT_KEYWORD(NAME)  \
   if (LangOpts.ObjC)           \
     AddObjCKeyword(StringRef(#NAME), tok::objc_##NAME, *this);
-#define INTERESTING_IDENTIFIER(NAME)                                           \
-  AddInterestingIdentifier(StringRef(#NAME), tok::NAME, *this);
+#define NOTABLE_IDENTIFIER(NAME)                                               \
+  AddNotableIdentifier(StringRef(#NAME), tok::NAME, *this);
 
 #define TESTING_KEYWORD(NAME, FLAGS)
 #include "clang/Basic/TokenKinds.def"
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index be23c0fffe0576..8c73c067c74c62 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -6839,21 +6839,21 @@ Sema::ActOnTypedefNameDecl(Scope *S, DeclContext *DC, TypedefNameDecl *NewTD,
   if (IdentifierInfo *II = NewTD->getIdentifier())
     if (!NewTD->isInvalidDecl() &&
         NewTD->getDeclContext()->getRedeclContext()->isTranslationUnit()) {
-      switch (II->getInterestingIdentifierID()) {
-      case tok::InterestingIdentifierKind::FILE:
+      switch (II->getNotableIdentifierID()) {
+      case tok::NotableIdentifierKind::FILE:
         Context.setFILEDecl(NewTD);
         break;
-      case tok::InterestingIdentifierKind::jmp_buf:
+      case tok::NotableIdentifierKind::jmp_buf:
         Context.setjmp_bufDecl(NewTD);
         break;
-      case tok::InterestingIdentifierKind::sigjmp_buf:
+      case tok::NotableIdentifierKind::sigjmp_buf:
         Context.setsigjmp_bufDecl(NewTD);
         break;
-      case tok::InterestingIdentifierKind::ucontext_t:
+      case tok::NotableIdentifierKind::ucontext_t:
         Context.setucontext_tDecl(NewTD);
         break;
-      case tok::InterestingIdentifierKind::float_t:
-      case tok::InterestingIdentifierKind::double_t:
+      case tok::NotableIdentifierKind::float_t:
+      case tok::NotableIdentifierKind::double_t:
         NewTD->addAttr(AvailableOnlyInDefaultEvalMethodAttr::Create(Context));
         break;
       default:
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index eea14a66fa1818..683a076e6bc399 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -988,8 +988,7 @@ ASTIdentifierLookupTraitBase::ReadKey(const unsigned char* d, unsigned n) {
 static bool isInterestingIdentifier(ASTReader &Reader, IdentifierInfo &II,
                                     bool IsModule) {
   bool IsInteresting =
-      II.getInterestingIdentifierID() !=
-          tok::InterestingIdentifierKind::not_interesting ||
+      II.getNotableIdentifierID() != tok::NotableIdentifierKind::not_notable ||
       II.getBuiltinID() != Builtin::ID::NotBuiltin ||
       II.getObjCKeywordID() != tok::ObjCKeywordKind::objc_not_keyword;
   return II.hadMacroDefinition() || II.isPoisoned() ||
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 7966b3175ec9f1..740bec586a5e33 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -3599,8 +3599,8 @@ class ASTIdentifierTableTrait {
   bool isInterestingIdentifier(const IdentifierInfo *II, uint64_t MacroOffset) {
     II->getObjCOrBuiltinID();
     bool IsInteresting =
-        II->getInterestingIdentifierID() !=
-            tok::InterestingIdentifierKind::not_interesting ||
+        II->getNotableIdentifierID() !=
+            tok::NotableIdentifierKind::not_notable ||
         II->getBuiltinID() != Builtin::ID::NotBuiltin ||
         II->getObjCKeywordID() != tok::ObjCKeywordKind::objc_not_keyword;
     if (MacroOffset || II->isPoisoned() || (!IsModule && IsInteresting) ||

cor3ntin

LGTM
You might want to put a [NFC] in the commit message

AaronBallman

LGTM, but agreed on adding [NFC] to the patch title

Endilll requested review from cor3ntin and AaronBallman February 12, 2024 22:00

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels Feb 12, 2024

cor3ntin approved these changes Feb 13, 2024

View reviewed changes

AaronBallman approved these changes Feb 13, 2024

View reviewed changes

Endilll changed the title ~~[clang] Use "notable" for "interesting" identifiers in IdentifierInfo~~ [clang][NFC] Use "notable" for "interesting" identifiers in IdentifierInfo Feb 14, 2024

Endilll merged commit 5027569 into llvm:main Feb 14, 2024
8 checks passed

Endilll deleted the interesting-identifier branch February 14, 2024 12:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clang][NFC] Use "notable" for "interesting" identifiers in `IdentifierInfo` #81542

[clang][NFC] Use "notable" for "interesting" identifiers in `IdentifierInfo` #81542

Endilll commented Feb 12, 2024 •

edited

llvmbot commented Feb 12, 2024 •

edited

cor3ntin left a comment

AaronBallman left a comment

[clang][NFC] Use "notable" for "interesting" identifiers in IdentifierInfo #81542

[clang][NFC] Use "notable" for "interesting" identifiers in IdentifierInfo #81542

Conversation

Endilll commented Feb 12, 2024 • edited

llvmbot commented Feb 12, 2024 • edited

cor3ntin left a comment

Choose a reason for hiding this comment

AaronBallman left a comment

Choose a reason for hiding this comment

[clang][NFC] Use "notable" for "interesting" identifiers in `IdentifierInfo` #81542

[clang][NFC] Use "notable" for "interesting" identifiers in `IdentifierInfo` #81542

Endilll commented Feb 12, 2024 •

edited

llvmbot commented Feb 12, 2024 •

edited