Skip to content

Commit 45066b9

Browse files
smeenaitru
authored andcommitted
[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 #63462 (cherry picked from commit 61c5ad8)
1 parent 87ec1f4 commit 45066b9

File tree

2 files changed

+50
-4
lines changed

2 files changed

+50
-4
lines changed

clang/lib/Sema/SemaChecking.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11308,7 +11308,11 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
1130811308
Hints.push_back(
1130911309
FixItHint::CreateInsertion(E->getBeginLoc(), CastFix.str()));
1131011310

11311-
SourceLocation After = S.getLocForEndOfToken(E->getEndLoc());
11311+
// We don't use getLocForEndOfToken because it returns invalid source
11312+
// locations for macro expansions (by design).
11313+
SourceLocation EndLoc = S.SourceMgr.getSpellingLoc(E->getEndLoc());
11314+
SourceLocation After = EndLoc.getLocWithOffset(
11315+
Lexer::MeasureTokenLength(EndLoc, S.SourceMgr, S.LangOpts));
1131211316
Hints.push_back(FixItHint::CreateInsertion(After, ")"));
1131311317
}
1131411318

clang/test/FixIt/format.cpp

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,61 @@
1+
// RUN: %clang_cc1 -fsyntax-only -verify -Wformat %s
12
// RUN: %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits -Wformat %s 2>&1 | FileCheck %s
23

34
extern "C" int printf(const char *, ...);
5+
#define LOG(...) printf(__VA_ARGS__)
46

57
namespace N {
68
enum class E { One };
79
}
810

9-
void a() {
11+
struct S {
12+
N::E Type;
13+
};
14+
15+
void a(N::E NEVal, S *SPtr, S &SRef) {
1016
printf("%d", N::E::One); // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}}
1117
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:16-[[@LINE-1]]:16}:"static_cast<int>("
1218
// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:25-[[@LINE-2]]:25}:")"
1319

14-
printf("%hd", N::E::One);
20+
printf("%hd", N::E::One); // expected-warning{{format specifies type 'short' but the argument has type 'N::E'}}
1521
// CHECK: "static_cast<short>("
1622

17-
printf("%hu", N::E::One);
23+
printf("%hu", N::E::One); // expected-warning{{format specifies type 'unsigned short' but the argument has type 'N::E'}}
1824
// CHECK: "static_cast<unsigned short>("
25+
26+
LOG("%d", N::E::One); // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}}
27+
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:13}:"static_cast<int>("
28+
// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:22-[[@LINE-2]]:22}:")"
29+
30+
printf("%d", NEVal); // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}}
31+
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:16-[[@LINE-1]]:16}:"static_cast<int>("
32+
// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:21-[[@LINE-2]]:21}:")"
33+
34+
LOG("%d", NEVal); // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}}
35+
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:13}:"static_cast<int>("
36+
// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:18-[[@LINE-2]]:18}:")"
37+
38+
printf(
39+
"%d",
40+
SPtr->Type // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}}
41+
);
42+
// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:7}:"static_cast<int>("
43+
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:17-[[@LINE-3]]:17}:")"
44+
45+
LOG( // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}}
46+
"%d",
47+
SPtr->Type
48+
);
49+
// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:7}:"static_cast<int>("
50+
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:17-[[@LINE-3]]:17}:")"
51+
52+
printf("%d",
53+
SRef.Type); // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}}
54+
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:"static_cast<int>("
55+
// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:")"
56+
57+
LOG("%d", // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}}
58+
SRef.Type);
59+
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:"static_cast<int>("
60+
// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:")"
1961
}

0 commit comments

Comments
 (0)