[clang][Sema] Add diagnostic note for reference of function-like macros requiring without parentheses #123495

StarOne01 · 2025-01-19T03:31:24Z

This PR enhances the Clang diagnostics to provide better guidance when function-like macros are used without parentheses. Additionally, it updates the relevant test cases to reflect these changes (#123038 ).

llvmbot · 2025-01-19T03:32:01Z

@llvm/pr-subscribers-clang

Author: Prashanth (StarOne01)

Changes

This PR enhances the Clang diagnostics to provide better guidance when function-like macros are used without parentheses. Additionally, it updates the relevant test cases to reflect these changes (#123038 ).

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

3 Files Affected:

(modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3)
(modified) clang/lib/Sema/SemaExpr.cpp (+14)
(modified) clang/test/Preprocessor/macro_with_initializer_list.cpp (+4-2)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index db54312ad965e8..37a9a67ac1efba 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5936,6 +5936,9 @@ def err_fold_expression_limit_exceeded: Error<
   "instantiating fold expression with %0 arguments exceeded expression nesting "
   "limit of %1">, DefaultFatal, NoSFINAE;
 
+def note_function_like_macro_requires_parens
+    : Note<"'%0' exists, but as a function-like macro; perhaps, did you forget "
+           "the parentheses?">;
 def err_unexpected_typedef : Error<
   "unexpected type name %0: expected expression">;
 def err_unexpected_namespace : Error<
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ae40895980d90a..44bab3e47233cb 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -2509,6 +2509,20 @@ bool Sema::DiagnoseEmptyLookup(Scope *S, CXXScopeSpec &SS, LookupResult &R,
     DC = DC->getLookupParent();
   }
 
+  // Check whether a similar function-like macro exists and suggest it
+  if (IdentifierInfo *II = Name.getAsIdentifierInfo()) {
+    if (II->hasMacroDefinition()) {
+      MacroInfo *MI = PP.getMacroInfo(II);
+      if (MI && MI->isFunctionLike()) {
+        Diag(R.getNameLoc(), diag::err_undeclared_var_use) << II->getName();
+        Diag(MI->getDefinitionLoc(),
+             diag::note_function_like_macro_requires_parens)
+            << II->getName();
+        return true;
+      }
+    }
+  }
+
   // We didn't find anything, so try to correct for a typo.
   TypoCorrection Corrected;
   if (S && Out) {
diff --git a/clang/test/Preprocessor/macro_with_initializer_list.cpp b/clang/test/Preprocessor/macro_with_initializer_list.cpp
index 40f53164b263d9..cc7dae0b5a3e00 100644
--- a/clang/test/Preprocessor/macro_with_initializer_list.cpp
+++ b/clang/test/Preprocessor/macro_with_initializer_list.cpp
@@ -134,6 +134,7 @@ void test_NE() {
 // CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{110:32-110:32}:")"
 
 #define INIT(var, init) Foo var = init; // expected-note 3{{defined here}}
+// expected-note@-1 2{{'INIT' exists, but as a function-like macro; perhaps, did you forget the parentheses?}}
 // Can't use an initializer list as a macro argument.  The commas in the list
 // will be interpretted as argument separaters and adding parenthesis will
 // make it no longer an initializer list.
@@ -159,12 +160,13 @@ void test() {
   // expected-note@-3 {{cannot use initializer list at the beginning of a macro argument}}
 }
 
-// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{145:11-145:11}:"("
-// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{145:23-145:23}:")"
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{146:11-146:11}:"("
+// CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{146:23-146:23}:")"
 
 #define M(name,a,b,c,d,e,f,g,h,i,j,k,l) \
   Foo name = a + b + c + d + e + f + g + h + i + j + k + l;
 // expected-note@-2 2{{defined here}}
+// expected-note@-3 {{'M' exists, but as a function-like macro; perhaps, did you forget the parentheses?}}
 void test2() {
   M(F1, Foo(), Foo(), Foo(), Foo(), Foo(), Foo(),
         Foo(), Foo(), Foo(), Foo(), Foo(), Foo());

Sirraide

This still needs tests and a release note.

Sirraide · 2025-01-19T23:38:24Z

clang/lib/Sema/SemaExpr.cpp

@@ -2509,6 +2509,20 @@ bool Sema::DiagnoseEmptyLookup(Scope *S, CXXScopeSpec &SS, LookupResult &R,
    DC = DC->getLookupParent();
  }

+  // Check whether a similar function-like macro exists and suggest it


Instead of doing this here before typo correction, the note that this pr adds should probably just be emitted right after the error we emit at the very end of this function (or after the one right before that we emit if the SS isn’t empty).

I did try it, but in this case the function returns here itself which is before giving up, maybe i could put the note inside this conditional?

I did try it, but in this case the function returns here itself

I'm not sure I understand what it is the problem with this return? It should happen when a typo replacement has been found, which seems not the case for the originally reported case, see https://godbolt.org/z/br9n6eGsT .
Could you please elaborate a bit more?

Because that

Instead of doing this here before typo correction, the note that this pr adds should probably just be emitted right after the error we emit at the very end of this function

Does seem like a correct soluion.

Hey @Fznamznon, thanks for asking, let me explain myself.

If i understand it right, that particular conditional statement should get executed for this:

int main(){ int var1 = 63; int out = var; }

and should be skipped for:

int main(){ int var1 = 63; int out = notFound; }

but the problem is that the conditional statement gets executed for both the cases, so when i put the new check somewhere below it, the control never reaches the new check.

please correct me if i'm off somewhere :)

Hmm, that sounds weird because I don’t think we should be doing any typo correction of notFound to var1 here. That said, I’m not too familiar w/ typo correction myself.

Sirraide · 2025-01-19T23:44:41Z

clang/test/Preprocessor/macro_with_initializer_list.cpp

@@ -134,6 +134,7 @@ void test_NE() {
 // CHECK: fix-it:"{{.*}}macro_with_initializer_list.cpp":{110:32-110:32}:")"

 #define INIT(var, init) Foo var = init; // expected-note 3{{defined here}}
+// expected-note@-1 2{{'INIT' exists, but as a function-like macro; perhaps, did you forget the parentheses?}}


Er, this note being emitted here is ... not great because we are using it (or at least trying to use it) as a function-like macro below—to be fair though, putting an init list in a function-like macro currently already leads to a flood of errors because we can’t recover from it too well, so if this is the only case that causes a false positive then it’s not that much of an issue imo.

clang/include/clang/Basic/DiagnosticSemaKinds.td

Fznamznon

Yeah, tests seem to be missing

clang/include/clang/Basic/DiagnosticSemaKinds.td

Fznamznon · 2025-01-20T13:37:21Z

clang/lib/Sema/SemaExpr.cpp

@@ -2509,6 +2509,20 @@ bool Sema::DiagnoseEmptyLookup(Scope *S, CXXScopeSpec &SS, LookupResult &R,
    DC = DC->getLookupParent();
  }

+  // Check whether a similar function-like macro exists and suggest it


I did try it, but in this case the function returns here itself

I'm not sure I understand what it is the problem with this return? It should happen when a typo replacement has been found, which seems not the case for the originally reported case, see https://godbolt.org/z/br9n6eGsT .
Could you please elaborate a bit more?

Because that

Instead of doing this here before typo correction, the note that this pr adds should probably just be emitted right after the error we emit at the very end of this function

Does seem like a correct soluion.

StarOne01 · 2025-01-21T11:51:34Z

Is there any other tests we would need to include?

Sirraide

The tests are in the wrong directory (you’ve added them to a file in test/Preprocessor, but this is a Sema change so the tests should probably be in test/Sema).

Additionally, a test involving typo correction would probably be a good idea:

#define FOO1() 
void f() {
    int FOO;
    int x = FOO1; // Typo correction to 'FOO' instead of note about 'FOO1()'.
}

StarOne01 · 2025-01-21T14:33:19Z

Thanks for the correction @Sirraide , yeah I forgot that area, will add.

StarOne01 · 2025-01-22T08:38:57Z

Hey @Sirraide , inorder to include the test, we would need to figure out what's going on with the *out pointer

shafik

Adding to the current comments.

shafik · 2025-01-31T23:22:03Z

clang/test/Preprocessor/macro_with_initializer_list.cpp


 #define M(name,a,b,c,d,e,f,g,h,i,j,k,l) \
  Foo name = a + b + c + d + e + f + g + h + i + j + k + l;
 // expected-note@-2 2{{defined here}}
+// expected-note@-3 {{'M' is defined here as a function-like macro; did you mean 'M(...)'}}


The addition of this diagnostic here when the diagnostic below is error: too many arguments provided to function-like macro invocation seems not very helpful and too verbose.

clang/test/Preprocessor/macro_with_initializer_list.cpp

StarOne01 · 2025-02-04T10:24:24Z

I did find what was wrong

When we try to correct typos, we seem to do it by looking over all the identifiers in the Context which includes the typo itself!

// clang/lib/Sema/SemaLookup.cpp
for (const auto &I : Context.Idents)
    Consumer->FoundName(I.getKey());

as the typo and the correction (the typo too) are identical, a correction is added.

This always happen if a typo is encountered.

To avoid this, i tried to check for identical identifiers, but in some cases identical identifiers are a necessary correction too for example when:

a unqualified identifier without the nested namespace.
a member call without it's obj, etc...

because of this ambiguity, the *out is always defined and non-empty when a typo is encountered, which is our problem here.

i'm lost rn, any help with fixing it would be appreciated!

…parentheses

… new behavior and Format the changes

…ut parentheses Co-authored-by: Sirraide <[email protected]>

…arify usage and improve error messages

…ut parentheses Co-authored-by: Mariya Podchishchaeva <[email protected]>

…he updated note

…nitializer list tests

mordante · 2025-02-09T14:57:30Z

Thanks @mordante, for explaining, but i don't think that i could remove a reviewer? Could i?

Do you have commit access for LLVM? If not you might indeed not be able to.

…function-like macros

StarOne01 · 2025-02-09T15:00:54Z

Do you have commit access for LLVM? If not you might indeed not be able to.

No, i don't have commit access.

mordante · 2025-02-09T15:14:09Z

I've removed the ones added by your merge.

Sirraide · 2025-02-10T09:07:10Z

Merging main into your branch tends to do that.

I’m not sure if this is the case, but my theory is that it’s specifically creating a new commit on merge that does that. After resolving conflicts, you have to run git merge --continue instead of committing manually. I always merge main instead of rebasing and I’ve never had any issues, but I’ve seen this happen to people several times.

Sirraide · 2025-02-10T09:08:06Z

Also, i see the typo correction is unpredictable as I addressed above, is that a expected behaviour?

Hmm, I don’t know too much about typo correction so I’m probably not the best person to look into that.

StarOne01 · 2025-02-10T12:28:35Z

Hmm, I don’t know too much about typo correction so I’m probably not the best person to look into that.

Could you tag someone who works on this?

clang/docs/ReleaseNotes.rst

clang/include/clang/Basic/DiagnosticSemaKinds.td

AaronBallman · 2025-02-10T17:35:52Z

clang/lib/Sema/SemaExpr.cpp

+      if (MI && MI->isFunctionLike()) {
+        SemaRef.Diag(TypoLoc,
+                     diag::err_undeclared_var_use_suggest_func_like_macro)
+            << II->getName();


Suggested change

<< II->getName();

<< II;

This will ensure the name is properly quoted by the diagnostics engine.

Tried it, but gives 'FOO1' is defined as an object-like macro; did you mean ''FOO1'(...)'? so reverted back.

clang/lib/Sema/SemaExpr.cpp

StarOne01 · 2025-02-11T02:22:41Z

Also, i see the typo correction is unpredictable as I addressed above, is that a expected behaviour?

@AaronBallman could you help me with this? I do understand the delayed typo thing but how is that being decided? also, I couldn't find that documented.

… related notes

…alizer list tests

StarOne01 · 2025-02-12T13:13:53Z

Done! (Sorry for force pushes 😅, git suddenly decided to act weird).

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 19, 2025

Sirraide reviewed Jan 19, 2025

View reviewed changes

Sirraide requested review from cor3ntin, AaronBallman and Fznamznon January 19, 2025 23:52

Fznamznon reviewed Jan 20, 2025

View reviewed changes

StarOne01 requested review from Fznamznon and Sirraide January 20, 2025 16:57

Sirraide reviewed Jan 21, 2025

View reviewed changes

shafik reviewed Jan 31, 2025

View reviewed changes

StarOne01 and others added 8 commits February 9, 2025 14:53

[clang][Sema] Add diagnostic note for function-like macros requiring …

ea6c9ca

…parentheses

[clang][Tests] Modify tests for function-like macros according to the…

c274178

… new behavior and Format the changes

Change the note for reference of function-like macros requiring witho…

33e001e

…ut parentheses Co-authored-by: Sirraide <[email protected]>

[clang][Tests] Update diagnostic tests for function-like macros to cl…

5aa4654

…arify usage and improve error messages

Change the note for reference of function-like macros requiring witho…

bd08487

…ut parentheses Co-authored-by: Mariya Podchishchaeva <[email protected]>

[clang][Tests] Update diagnostic tests for function-like macros for t…

6ef338a

…he updated note

[clang][Sema] Improve diagnostics for undeclared function-like macros

094887d

[clang][Tests] Enhance diagnostic notes for function-like macros in i…

eac145a

…nitializer list tests

StarOne01 requested review from daniel-grumberg, QuietMisdreavus, aaupov, maksfb, rafaelauler and ayermolo as code owners February 9, 2025 09:36

StarOne01 force-pushed the add_diag_for_funcLikeMacros branch from a7cb0e3 to eac145a Compare February 9, 2025 14:53

[clang][Docs] Add release note for diagnosing missing parentheses in …

e7404fb

…function-like macros

mordante removed request for a team, aaupov, QuietMisdreavus, dcci, bcardosolopes, maksfb, rafaelauler, lanza, Endilll, daniel-grumberg, yota9 and ayermolo February 9, 2025 15:13

AaronBallman reviewed Feb 10, 2025

View reviewed changes

[clang][Sema] Improve diagnostics for function-like macros and update…

e4d2186

… related notes

StarOne01 force-pushed the add_diag_for_funcLikeMacros branch from c0ed9df to d463086 Compare February 12, 2025 11:10

[clang][Sema] Update diagnostic tests for object-like macros in initi…

cc50469

…alizer list tests

StarOne01 force-pushed the add_diag_for_funcLikeMacros branch from d463086 to cc50469 Compare February 12, 2025 11:53

[clang][test]fix overlooked testcase

11adc17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clang][Sema] Add diagnostic note for reference of function-like macros requiring without parentheses #123495

[clang][Sema] Add diagnostic note for reference of function-like macros requiring without parentheses #123495

StarOne01 commented Jan 19, 2025

llvmbot commented Jan 19, 2025

Sirraide left a comment

Sirraide Jan 19, 2025

StarOne01 Jan 20, 2025

Fznamznon Jan 20, 2025

StarOne01 Jan 20, 2025 •

edited

Loading

Sirraide Jan 20, 2025

Sirraide Jan 19, 2025

Fznamznon left a comment

Fznamznon Jan 20, 2025

StarOne01 commented Jan 21, 2025

Sirraide left a comment •

edited

Loading

StarOne01 commented Jan 21, 2025

StarOne01 commented Jan 22, 2025

shafik left a comment

shafik Jan 31, 2025

StarOne01 commented Feb 4, 2025 •

edited

Loading

mordante commented Feb 9, 2025

StarOne01 commented Feb 9, 2025

mordante commented Feb 9, 2025

Sirraide commented Feb 10, 2025

Sirraide commented Feb 10, 2025

StarOne01 commented Feb 10, 2025

AaronBallman Feb 10, 2025

StarOne01 Feb 12, 2025

StarOne01 commented Feb 11, 2025 •

edited

Loading

StarOne01 commented Feb 12, 2025

[clang][Sema] Add diagnostic note for reference of function-like macros requiring without parentheses #123495

Are you sure you want to change the base?

[clang][Sema] Add diagnostic note for reference of function-like macros requiring without parentheses #123495

Conversation

StarOne01 commented Jan 19, 2025

llvmbot commented Jan 19, 2025

Sirraide left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StarOne01 Jan 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fznamznon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StarOne01 commented Jan 21, 2025

Sirraide left a comment • edited Loading

Choose a reason for hiding this comment

StarOne01 commented Jan 21, 2025

StarOne01 commented Jan 22, 2025

shafik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StarOne01 commented Feb 4, 2025 • edited Loading

mordante commented Feb 9, 2025

StarOne01 commented Feb 9, 2025

mordante commented Feb 9, 2025

Sirraide commented Feb 10, 2025

Sirraide commented Feb 10, 2025

StarOne01 commented Feb 10, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StarOne01 commented Feb 11, 2025 • edited Loading

StarOne01 commented Feb 12, 2025

StarOne01 Jan 20, 2025 •

edited

Loading

Sirraide left a comment •

edited

Loading

StarOne01 commented Feb 4, 2025 •

edited

Loading

StarOne01 commented Feb 11, 2025 •

edited

Loading