Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Expression end location not working in macros #63462

Closed
abrachet opened this issue Jun 23, 2023 · 4 comments · Fixed by #66853 or llvm/llvm-project-release-prs#704
Closed

Expression end location not working in macros #63462

abrachet opened this issue Jun 23, 2023 · 4 comments · Fixed by #66853 or llvm/llvm-project-release-prs#704
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema"
Milestone
LLVM 17.0.X Release

Comments

@abrachet
Copy link
Member

abrachet commented Jun 23, 2023 •
edited by VoltrexKeyva
Loading 

#define MACRO(var) {var}

char ok(int a) {
    return {a}; // static_cast<char>( )
}

char macro(int a) {
    return MACRO(a); // static_cast<char>(
    // note the lack of end `)`
}

This fixit is issued here

llvm-project/clang/lib/Sema/SemaInit.cpp

Line 10366 in 7175d6a

<< FixItHint::CreateInsertion(

and uses FixItHint::CreateInsertion(S.getLocForEndOfToken(PostInit->getEndLoc()), ")"); to emit the insertion for the end paren. It is just missing in the fixit hint.
@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" and removed new issue labels Jun 23, 2023
@llvmbot
Copy link
Member

llvmbot commented Jun 23, 2023

@llvm/issue-subscribers-clang-frontend

@smeenai smeenai mentioned this issue Sep 20, 2023
Merged
smeenai added a commit to smeenai/llvm-project that referenced this issue Sep 20, 2023
@smeenai
[Sema] Fix fixit cast printing inside macros
701b1d9 
`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 };
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
smeenai added a commit that referenced this issue Sep 21, 2023
@smeenai
[Sema] Fix fixit cast printing inside macros (#66853)

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
61c5ad8 
`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
@smeenai smeenai added this to the LLVM 17.0.X Release milestone Sep 21, 2023
@smeenai
Copy link
Collaborator

smeenai commented Sep 21, 2023

/cherry-pick 61c5ad8

@llvmbot
Copy link
Member

llvmbot commented Sep 21, 2023

/branch llvm/llvm-project-release-prs/issue63462

llvmbot pushed a commit to llvm/llvm-project-release-prs that referenced this issue Sep 21, 2023
@smeenai @llvmbot
[Sema] Fix fixit cast printing inside macros (#66853)
b486839 
`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/llvm-project#63462

(cherry picked from commit 61c5ad8857a71510e4393680a1e42740da4ba6fa)
@llvmbot llvmbot mentioned this issue Sep 21, 2023
Merged
@llvmbot
Copy link
Member

llvmbot commented Sep 21, 2023

/pull-request llvm/llvm-project-release-prs#704

@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Sep 28, 2023
@tru tru moved this from Needs Triage to Needs Review in LLVM Release Status Sep 28, 2023
tru pushed a commit to llvm/llvm-project-release-prs that referenced this issue Sep 29, 2023
@smeenai @tru
[Sema] Fix fixit cast printing inside macros (#66853)
45066b9 
`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/llvm-project#63462

(cherry picked from commit 61c5ad8857a71510e4393680a1e42740da4ba6fa)
@tru tru moved this from Needs Review to Done in LLVM Release Status Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema"
Projects
LLVM Release Status
Status: Done
Milestone
LLVM 17.0.X Release
4 participants
@smeenai @abrachet @EugeneZelenko @llvmbot