Skip to content

[clang] Fixes compile error that double colon operator cannot resolve macro with parentheses. #68618

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.

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

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

lygstate
Copy link
Contributor

@lygstate lygstate commented Oct 9, 2023

Error message:

In file included from ../src/amd/addrlib/src/core/addrobject.h:21:
../src/amd/addrlib/src/core/addrcommon.h:343:13: error: expected unqualified-id
    out = ::_tzcnt_u32(mask);
            ^
/usr/lib/llvm-15/lib/clang/15.0.6/include/bmiintrin.h:74:27: note: expanded from macro '_tzcnt_u32'
#define _tzcnt_u32(a)     (__tzcnt_u32((a)))

This is because both GCC/Clang doesn't support compiling the following code:

#ifdef _MSC_VER
#include <intrin.h>
#else
#include <x86intrin.h>
#endif

int f(int a) {
    return ::(_tzcnt_u32)(a);
}

This is because the return statement expects an expression or braced init list: http://eel.is/c++draft/stmt.jump#general-1 but we really only need to care about the expression form (there's no braces in sight). Grammatically, the leading :: will parse as a qualified-id because it matches the production for nested-name-specifier: http://eel.is/c++draft/expr.prim.id.qual#nt:qualified-id That needs to be followed by an unqualified-id (http://eel.is/c++draft/expr.prim.id.unqual#nt:unqualified-id), but the open paren does not match any of the grammar productions, so this is a syntax error.

Closes: #64467

@lygstate
Copy link
Contributor Author

lygstate commented Oct 9, 2023

it's already accepted in https://reviews.llvm.org/D157297

@llvmbot llvmbot added clang Clang issues not falling into any other category backend:X86 clang:headers Headers provided by Clang, e.g. for intrinsics labels Oct 9, 2023
@lygstate
Copy link
Contributor Author

lygstate commented Nov 7, 2023

ping for merge @phoebewang @AaronBallman

Copy link
Contributor

@phoebewang phoebewang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@AaronBallman AaronBallman self-requested a review November 8, 2023 16:47
Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but please add a release note about the fix to clang/docs/ReleaseNotes.rst

@lygstate lygstate force-pushed the main branch 2 times, most recently from 63c1f1b to 6a8fd54 Compare November 12, 2023 05:47
@lygstate
Copy link
Contributor Author

@AaronBallman is this OK ?

… macro with parentheses.

Error message:
```
In file included from ../src/amd/addrlib/src/core/addrobject.h:21:
../src/amd/addrlib/src/core/addrcommon.h:343:13: error: expected unqualified-id
    out = ::_tzcnt_u32(mask);
            ^
/usr/lib/llvm-15/lib/clang/15.0.6/include/bmiintrin.h:74:27: note: expanded from macro '_tzcnt_u32'
#define _tzcnt_u32(a)     (__tzcnt_u32((a)))
```

This is because both GCC/Clang doesn't support compiling the following code:
```
#ifdef _MSC_VER
#include <intrin.h>
#else
#include <x86intrin.h>
#endif

int f(int a) {
    return ::(_tzcnt_u32)(a);
}
```

This is because the return statement expects an expression or braced init list: http://eel.is/c++draft/stmt.jump#general-1 but we really only need to care about the expression form (there's no braces in sight).
Grammatically, the leading :: will parse as a qualified-id because it matches the production for nested-name-specifier: http://eel.is/c++draft/expr.prim.id.qual#nt:qualified-id
That needs to be followed by an unqualified-id (http://eel.is/c++draft/expr.prim.id.unqual#nt:unqualified-id), but the open paren does not match any of the grammar productions, so this is a syntax error.

Closes: llvm#64467

Signed-off-by: Yonggang Luo <[email protected]>
@lygstate
Copy link
Contributor Author

lygstate commented Dec 4, 2023

ping for merge

@phoebewang phoebewang merged commit 8cfdd37 into llvm:main Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 clang:headers Headers provided by Clang, e.g. for intrinsics clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

/usr/lib/llvm-15/lib/clang/15.0.6/include/bmiintrin.h:74:27: note: expanded from macro '_tzcnt_u32'
4 participants