Skip to content

Commit

Permalink
[Sema] Fix fixit cast printing inside macros (llvm#66853)
Browse files Browse the repository at this point in the history
`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 llvm#63462

(cherry picked from commit 61c5ad8)
  • Loading branch information
smeenai authored and tru committed Sep 29, 2023
1 parent 87ec1f4 commit 45066b9
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 4 deletions.
6 changes: 5 additions & 1 deletion clang/lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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, ")"));
}

Expand Down
48 changes: 45 additions & 3 deletions clang/test/FixIt/format.cpp
Original file line number Diff line number Diff line change
@@ -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}:")"
}

0 comments on commit 45066b9

Please sign in to comment.