[Sema] Fix fixit cast printing inside macros

[smeenai]

Lexer::getLocForEndOfToken is documented as returning an invalid
source location when the end of the token is inside a macro expansion.
We don't want that for this particular application, so just calculate
the end location directly instead.

Before this, format fix-its would omit the closing parenthesis (thus
producing invalid code) for macros, e.g.:

$ cat format.cpp
extern "C" int printf(const char *, ...);
enum class Foo { Bar };
#define LOG(...) printf(__VA_ARGS__)
void f(Foo foo) { LOG("%d\n", foo); }

$ clang -fsyntax-only format.cpp
format.cpp:4:29: warning: format specifies type 'int' but the argument has type 'Foo' [-Wformat]
    4 | void f(Foo f) { LOG("%d\n", f); }
      |                      ~~     ^
      |                             static_cast<int>(
format.cpp:3:25: note: expanded from macro 'LOG'
    3 | #define LOG(...) printf(__VA_ARGS__)
      |                         ^~~~~~~~~~~
1 warning generated.

We now emit a valid fix-it:

$ clang -fsyntax-only format.cpp
format.cpp:4:31: warning: format specifies type 'int' but the argument has type 'Foo' [-Wformat]
    4 | void f(Foo foo) { LOG("%d\n", foo); }
      |                        ~~     ^~~
      |                               static_cast<int>( )
format.cpp:3:25: note: expanded from macro 'LOG'
    3 | #define LOG(...) printf(__VA_ARGS__)
      |                         ^~~~~~~~~~~
1 warning generated.

Fixes #63462

[llvmbot]

@llvm/pr-subscribers-clang

Changes

Lexer::getLocForEndOfToken is documented as returning an invalid
source location when the end of the token is inside a macro expansion.
We don't want that for this particular application, so just calculate
the end location directly instead.

Fixes #63462

---

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

2 Files Affected:

• (modified) clang/lib/Sema/SemaChecking.cpp (+4-1)
• (modified) clang/test/FixIt/format.cpp (+5)

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index fad70223362eddd..4fbdb315ddc87b4 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -11470,7 +11470,10 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
         Hints.push_back(
             FixItHint::CreateInsertion(E->getBeginLoc(), CastFix.str()));
 
-        SourceLocation After = S.getLocForEndOfToken(E->getEndLoc());
+        // We don't use getLocForEndOfToken because it returns invalid source
+        // locations for macro expansions (by design).
+        SourceLocation After = E->getEndLoc().getLocWithOffset(
+            Lexer::MeasureTokenLength(E->getEndLoc(), S.SourceMgr, S.LangOpts));
         Hints.push_back(FixItHint::CreateInsertion(After, ")"));
       }
 
diff --git a/clang/test/FixIt/format.cpp b/clang/test/FixIt/format.cpp
index 9cc4c2600eb6670..46a0a0a11cc850a 100644
--- a/clang/test/FixIt/format.cpp
+++ b/clang/test/FixIt/format.cpp
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits -Wformat %s 2>&1 | FileCheck %s
 
 extern "C" int printf(const char *, ...);
+#define LOG(...) printf(__VA_ARGS__)
 
 namespace N {
   enum class E { One };
@@ -16,4 +17,8 @@ void a() {
 
   printf("%hu", N::E::One);
   // CHECK: "static_cast<unsigned short>("
+
+  LOG("%d", N::E::One); // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:13}:"static_cast<int>("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:22-[[@LINE-2]]:22}:")"
 }

[tbaederr]

Since this is a diagnostic improvement, can you post the before and after output of an example?

[smeenai]

I updated the PR description with an example.

[abrachet]

Awesome! I couldn't figure this out when I looked :)