Skip to content

[clang][Lex][NFC] Reorder SrcMgr checks in CheckMacroName #141483

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 29, 2025

Conversation

tbaederr
Copy link
Contributor

isInPredefinedFile() will look at the presumed loc, which is comparatively slow. Move it after isInSystemFile().

http://llvm-compile-time-tracker.com/compare.php?from=843e362318e884991e517a54446b4faeacdad789&to=de0421a1a38052042721a67a6094f5cb38431f26&stat=instructions:u

isInPredefinedFile() will look at the presumed loc, which is
comparatively slow. Move it after isInSystemFile().
@tbaederr tbaederr requested a review from cor3ntin May 26, 2025 12:07
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 26, 2025
@llvmbot
Copy link
Member

llvmbot commented May 26, 2025

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

isInPredefinedFile() will look at the presumed loc, which is comparatively slow. Move it after isInSystemFile().

http://llvm-compile-time-tracker.com/compare.php?from=843e362318e884991e517a54446b4faeacdad789&to=de0421a1a38052042721a67a6094f5cb38431f26&stat=instructions:u


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

1 Files Affected:

  • (modified) clang/lib/Lex/PPDirectives.cpp (+2-2)
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index b2a8459d6b9cc..12688f0939a26 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -373,8 +373,8 @@ bool Preprocessor::CheckMacroName(Token &MacroNameTok, MacroUse isDefineUndef,
   // Macro names with reserved identifiers are accepted if built-in or passed
   // through the command line (the later may be present if -dD was used to
   // generate the preprocessed file).
-  if (!SourceMgr.isInPredefinedFile(MacroNameLoc) &&
-      !SourceMgr.isInSystemHeader(MacroNameLoc)) {
+  if (!SourceMgr.isInSystemHeader(MacroNameLoc) &&
+      !SourceMgr.isInPredefinedFile(MacroNameLoc)) {
     MacroDiag D = MD_NoWarn;
     if (isDefineUndef == MU_Define) {
       D = shouldWarnOnMacroDef(*this, II);

@@ -373,8 +373,8 @@ bool Preprocessor::CheckMacroName(Token &MacroNameTok, MacroUse isDefineUndef,
// Macro names with reserved identifiers are accepted if built-in or passed
// through the command line (the later may be present if -dD was used to
// generate the preprocessed file).
if (!SourceMgr.isInPredefinedFile(MacroNameLoc) &&
!SourceMgr.isInSystemHeader(MacroNameLoc)) {
if (!SourceMgr.isInSystemHeader(MacroNameLoc) &&
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like change deserves a note since it is not obvious the order matters or why.

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Impressive gain!

@tbaederr tbaederr merged commit 5a7931f into llvm:main May 29, 2025
11 checks passed
google-yfyang pushed a commit to google-yfyang/llvm-project that referenced this pull request May 29, 2025
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants