[NFC] Move RegisterBuiltinMacro function into the Preprocessor class

[muiez]

The RegisterBuiltinMacro function has been moved to the Preprocessor class for accessibility and has been refactored for readability.

[llvmbot]

@llvm/pr-subscribers-clang

Author: None (muiez)

Changes

The RegisterBuiltinMacro function has been moved to the Preprocessor class for accessibility and has been refactored for readability.

---

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

2 Files Affected:

• (modified) clang/include/clang/Lex/Preprocessor.h (+13)
• (modified) clang/lib/Lex/PPMacroExpansion.cpp (+34-47)

diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index fc7d0053f2323..9247323b107ff 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -2612,6 +2612,19 @@ class Preprocessor {
   /// \#pragma GCC poison/system_header/dependency and \#pragma once.
   void RegisterBuiltinPragmas();
 
+  /// RegisterBuiltinMacro - Register the specified identifier in the identifier
+  /// table and mark it as a builtin macro to be expanded.
+  IdentifierInfo *RegisterBuiltinMacro(const char *Name){
+    // Get the identifier.
+    IdentifierInfo *Id = getIdentifierInfo(Name);
+
+    // Mark it as being a macro that is builtin.
+    MacroInfo *MI = AllocateMacroInfo(SourceLocation());
+    MI->setIsBuiltinMacro();
+    appendDefMacroDirective(Id, MI);
+    return Id;
+  }
+
   /// Register builtin macros such as __LINE__ with the identifier table.
   void RegisterBuiltinMacros();
 
diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp
index 3913ff08c2eb5..4b82c14a917c3 100644
--- a/clang/lib/Lex/PPMacroExpansion.cpp
+++ b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -321,84 +321,71 @@ void Preprocessor::dumpMacroInfo(const IdentifierInfo *II) {
   }
 }
 
-/// RegisterBuiltinMacro - Register the specified identifier in the identifier
-/// table and mark it as a builtin macro to be expanded.
-static IdentifierInfo *RegisterBuiltinMacro(Preprocessor &PP, const char *Name){
-  // Get the identifier.
-  IdentifierInfo *Id = PP.getIdentifierInfo(Name);
-
-  // Mark it as being a macro that is builtin.
-  MacroInfo *MI = PP.AllocateMacroInfo(SourceLocation());
-  MI->setIsBuiltinMacro();
-  PP.appendDefMacroDirective(Id, MI);
-  return Id;
-}
-
 /// RegisterBuiltinMacros - Register builtin macros, such as __LINE__ with the
 /// identifier table.
 void Preprocessor::RegisterBuiltinMacros() {
-  Ident__LINE__ = RegisterBuiltinMacro(*this, "__LINE__");
-  Ident__FILE__ = RegisterBuiltinMacro(*this, "__FILE__");
-  Ident__DATE__ = RegisterBuiltinMacro(*this, "__DATE__");
-  Ident__TIME__ = RegisterBuiltinMacro(*this, "__TIME__");
-  Ident__COUNTER__ = RegisterBuiltinMacro(*this, "__COUNTER__");
-  Ident_Pragma  = RegisterBuiltinMacro(*this, "_Pragma");
-  Ident__FLT_EVAL_METHOD__ = RegisterBuiltinMacro(*this, "__FLT_EVAL_METHOD__");
+  Ident__LINE__ = RegisterBuiltinMacro("__LINE__");
+  Ident__FILE__ = RegisterBuiltinMacro("__FILE__");
+  Ident__DATE__ = RegisterBuiltinMacro("__DATE__");
+  Ident__TIME__ = RegisterBuiltinMacro("__TIME__");
+  Ident__COUNTER__ = RegisterBuiltinMacro("__COUNTER__");
+  Ident_Pragma  = RegisterBuiltinMacro("_Pragma");
+  Ident__FLT_EVAL_METHOD__ = RegisterBuiltinMacro("__FLT_EVAL_METHOD__");
 
   // C++ Standing Document Extensions.
   if (getLangOpts().CPlusPlus)
     Ident__has_cpp_attribute =
-        RegisterBuiltinMacro(*this, "__has_cpp_attribute");
+        RegisterBuiltinMacro("__has_cpp_attribute");
   else
     Ident__has_cpp_attribute = nullptr;
 
   // GCC Extensions.
-  Ident__BASE_FILE__     = RegisterBuiltinMacro(*this, "__BASE_FILE__");
-  Ident__INCLUDE_LEVEL__ = RegisterBuiltinMacro(*this, "__INCLUDE_LEVEL__");
-  Ident__TIMESTAMP__     = RegisterBuiltinMacro(*this, "__TIMESTAMP__");
+  Ident__BASE_FILE__     = RegisterBuiltinMacro("__BASE_FILE__");
+  Ident__INCLUDE_LEVEL__ = RegisterBuiltinMacro("__INCLUDE_LEVEL__");
+  Ident__TIMESTAMP__     = RegisterBuiltinMacro("__TIMESTAMP__");
 
   // Microsoft Extensions.
   if (getLangOpts().MicrosoftExt) {
-    Ident__identifier = RegisterBuiltinMacro(*this, "__identifier");
-    Ident__pragma = RegisterBuiltinMacro(*this, "__pragma");
+    Ident__identifier = RegisterBuiltinMacro("__identifier");
+    Ident__pragma = RegisterBuiltinMacro("__pragma");
   } else {
     Ident__identifier = nullptr;
     Ident__pragma = nullptr;
   }
 
   // Clang Extensions.
-  Ident__FILE_NAME__      = RegisterBuiltinMacro(*this, "__FILE_NAME__");
-  Ident__has_feature      = RegisterBuiltinMacro(*this, "__has_feature");
-  Ident__has_extension    = RegisterBuiltinMacro(*this, "__has_extension");
-  Ident__has_builtin      = RegisterBuiltinMacro(*this, "__has_builtin");
+  Ident__FILE_NAME__      = RegisterBuiltinMacro("__FILE_NAME__");
+  Ident__has_feature      = RegisterBuiltinMacro("__has_feature");
+  Ident__has_extension    = RegisterBuiltinMacro("__has_extension");
+  Ident__has_builtin      = RegisterBuiltinMacro("__has_builtin");
   Ident__has_constexpr_builtin =
-      RegisterBuiltinMacro(*this, "__has_constexpr_builtin");
-  Ident__has_attribute    = RegisterBuiltinMacro(*this, "__has_attribute");
+      RegisterBuiltinMacro("__has_constexpr_builtin");
+  Ident__has_attribute    = RegisterBuiltinMacro("__has_attribute");
   if (!getLangOpts().CPlusPlus)
-    Ident__has_c_attribute = RegisterBuiltinMacro(*this, "__has_c_attribute");
+    Ident__has_c_attribute = RegisterBuiltinMacro("__has_c_attribute");
   else
     Ident__has_c_attribute = nullptr;
 
-  Ident__has_declspec = RegisterBuiltinMacro(*this, "__has_declspec_attribute");
-  Ident__has_embed = RegisterBuiltinMacro(*this, "__has_embed");
-  Ident__has_include      = RegisterBuiltinMacro(*this, "__has_include");
-  Ident__has_include_next = RegisterBuiltinMacro(*this, "__has_include_next");
-  Ident__has_warning      = RegisterBuiltinMacro(*this, "__has_warning");
-  Ident__is_identifier    = RegisterBuiltinMacro(*this, "__is_identifier");
-  Ident__is_target_arch   = RegisterBuiltinMacro(*this, "__is_target_arch");
-  Ident__is_target_vendor = RegisterBuiltinMacro(*this, "__is_target_vendor");
-  Ident__is_target_os     = RegisterBuiltinMacro(*this, "__is_target_os");
+  Ident__has_declspec = RegisterBuiltinMacro("__has_declspec_attribute");
+  Ident__has_embed = RegisterBuiltinMacro("__has_embed");
+  Ident__has_include      = RegisterBuiltinMacro("__has_include");
+  Ident__has_include_next = RegisterBuiltinMacro("__has_include_next");
+  Ident__has_warning      = RegisterBuiltinMacro("__has_warning");
+  Ident__is_identifier    = RegisterBuiltinMacro("__is_identifier");
+  Ident__is_target_arch   = RegisterBuiltinMacro("__is_target_arch");
+  Ident__is_target_vendor = RegisterBuiltinMacro("__is_target_vendor");
+  Ident__is_target_os     = RegisterBuiltinMacro("__is_target_os");
   Ident__is_target_environment =
-      RegisterBuiltinMacro(*this, "__is_target_environment");
+      RegisterBuiltinMacro("__is_target_environment");
   Ident__is_target_variant_os =
-      RegisterBuiltinMacro(*this, "__is_target_variant_os");
+      RegisterBuiltinMacro("__is_target_variant_os");
   Ident__is_target_variant_environment =
-      RegisterBuiltinMacro(*this, "__is_target_variant_environment");
+      RegisterBuiltinMacro("__is_target_variant_environment");
 
   // Modules.
-  Ident__building_module  = RegisterBuiltinMacro(*this, "__building_module");
+  Ident__building_module  = RegisterBuiltinMacro("__building_module");
   if (!getLangOpts().CurrentModule.empty())
-    Ident__MODULE__ = RegisterBuiltinMacro(*this, "__MODULE__");
+    Ident__MODULE__ = RegisterBuiltinMacro("__MODULE__");
   else
     Ident__MODULE__ = nullptr;
 }

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[Sirraide]

This does feel like it could be useful, but do you need this (or anticipate that you’ll be needing this) for anything in particular elswhere?

[muiez]

This does feel like it could be useful, but do you need this (or anticipate that you’ll be needing this) for anything in particular elswhere?

Yes, we will need this to conditionally register a builtin macro during the Lexing phase, to be expanded correctly during the preprocessor output (with -E). In particular, we anticipate having a builtin macro that should only be defined when a specific pragma is defined. This change allows for that, and improves code quality.

[shafik]

Seems sensible @cor3ntin wdyt?

[muiez]

Gentle ping for review :)

[muiez]

Can I get a round of reviews please?

[abhina-sree]

LGTM