diff options
author | Shoaib Meenai <smeenai@fb.com> | 2023-09-20 17:32:35 -0700 |
---|---|---|
committer | Tobias Hieta <tobias@hieta.se> | 2023-09-29 08:23:57 +0200 |
commit | 45066b9fbc7b84dfcf5363d01fadd3d42c049e7f (patch) | |
tree | 4a56200ea5c9b692c8fbaf09195e1ce31180f088 | |
parent | Work around two more instances of __noinline__ conflicts. (#66138) (diff) | |
download | llvm-project-45066b9fbc7b84dfcf5363d01fadd3d42c049e7f.tar.gz llvm-project-45066b9fbc7b84dfcf5363d01fadd3d42c049e7f.tar.bz2 llvm-project-45066b9fbc7b84dfcf5363d01fadd3d42c049e7f.zip |
[Sema] Fix fixit cast printing inside macros (#66853)
`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 https://github.com/llvm/llvm-project/issues/63462
(cherry picked from commit 61c5ad8857a71510e4393680a1e42740da4ba6fa)
-rw-r--r-- | clang/lib/Sema/SemaChecking.cpp | 6 | ||||
-rw-r--r-- | clang/test/FixIt/format.cpp | 48 |
2 files changed, 50 insertions, 4 deletions
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index f8e48728da66..8626fc6ea16f 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -11308,7 +11308,11 @@ 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 EndLoc = S.SourceMgr.getSpellingLoc(E->getEndLoc()); + SourceLocation After = EndLoc.getLocWithOffset( + Lexer::MeasureTokenLength(EndLoc, 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 9cc4c2600eb6..5016ee587ed1 100644 --- a/clang/test/FixIt/format.cpp +++ b/clang/test/FixIt/format.cpp @@ -1,19 +1,61 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wformat %s // 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 }; } -void a() { +struct S { + N::E Type; +}; + +void a(N::E NEVal, S *SPtr, S &SRef) { printf("%d", N::E::One); // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}} // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:16-[[@LINE-1]]:16}:"static_cast<int>(" // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:25-[[@LINE-2]]:25}:")" - printf("%hd", N::E::One); + printf("%hd", N::E::One); // expected-warning{{format specifies type 'short' but the argument has type 'N::E'}} // CHECK: "static_cast<short>(" - printf("%hu", N::E::One); + printf("%hu", N::E::One); // expected-warning{{format specifies type 'unsigned short' but the argument has type 'N::E'}} // 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}:")" + + printf("%d", NEVal); // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:16-[[@LINE-1]]:16}:"static_cast<int>(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:21-[[@LINE-2]]:21}:")" + + LOG("%d", NEVal); // 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]]:18-[[@LINE-2]]:18}:")" + + printf( + "%d", + SPtr->Type // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}} + ); + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:7}:"static_cast<int>(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:17-[[@LINE-3]]:17}:")" + + LOG( // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}} + "%d", + SPtr->Type + ); + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:7}:"static_cast<int>(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:17-[[@LINE-3]]:17}:")" + + printf("%d", + SRef.Type); // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:"static_cast<int>(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:")" + + LOG("%d", // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}} + SRef.Type); + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:"static_cast<int>(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:")" } |