llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.8k stars 11.9k forks source link

Need __has_pragma #51887

Open llvmbot opened 2 years ago

llvmbot commented 2 years ago
Bugzilla Link 52545
Version unspecified
OS All
Reporter LLVM Bugzilla Contributor
CC @AaronBallman,@zygoloid

Extended Description

We have has_feature and __has_attribute and a number of other variants so that code can take advantage of new features while still functioning with older clang versions. We should also have has_pragma for the same reason.

In particular, I was recently bitten by trying to use the clang fp reassociate(on) pragma, which is available in clang-11+; rather than producing an "unknown pragma" warning, older clangs produce the error "invalid option 'reassociate'; expected contract".

An alternative solution (which would have worked in my case) would be to have unknown pragma options always be a warning instead of an error, but in general it's a better option to have __has_pragma, because it gives the user the building block to fallback on a different solution or to produce an error in cases where that's more appropriate.

I think a minimum-viable __has_pragma would take a string argument that's the entire pragma body and produce a result of 1 if the string is an accepted pragma for the current target.

AaronBallman commented 2 years ago

One oddity about this design is that it does not behave like other feature test macros. For example, we do not support __has_cpp_attribute(nodiscard("reason")) as the syntax, you only specify the name of the attribute.

Because doing that would put us back in the same situation. Someone will add a new option to an existing pragma, and we'll have no mechanism to test if it can be used.

You can test the value returned, same as __has_cpp_attribute. It's a bit annoying because there's no name to associate with that date value, but it is a progressing value that can be used to differentiate.

llvmbot commented 2 years ago

One oddity about this design is that it does not behave like other feature test macros. For example, we do not support __has_cpp_attribute(nodiscard("reason")) as the syntax, you only specify the name of the attribute.

Because doing that would put us back in the same situation. Someone will add a new option to an existing pragma, and we'll have no mechanism to test if it can be used.

AaronBallman commented 2 years ago

We have has_feature and __has_attribute and a number of other variants so that code can take advantage of new features while still functioning with older clang versions. We should also have has_pragma for the same reason.

Agreed. Morally, __has_pragma is similar to all the other feature testing macros. And Clang (downstreams) support an ever-growing set of implementation-defined pragmas, so this has significant utility in writing portable code within Clang and downstream compilers.

I think a minimum-viable __has_pragma would take a string argument that's the entire pragma body and produce a result of 1 if the string is an accepted pragma for the current target.

Why a string argument instead of pp-tokens?

Also, by "accepted", I assume you mean "syntactically valid" and not "semantically valid" (e.g., we're not going to try to tell you that the way you're USING pragma is supported, just that the way you're SPELLING the pragma is known to the compiler, right?)

One oddity about this design is that it does not behave like other feature test macros. For example, we do not support __has_cpp_attribute(nodiscard("reason")) as the syntax, you only specify the name of the attribute. Pragmas are a bit weirder in that they're an arbitrary hodge podge of pp-tokens, but my initial inclination was that __has_pragma would only need as many tokens as it requires to figure out the "name" of the pragma and then returns an integer "date value" like the other feature testing macros. This might work for Clang, but it might not work for other compilers if they have really weird pragma forms like #pragma 12.

llvmbot commented 1 year ago

@llvm/issue-subscribers-clang-frontend

| | | | --- | --- | | Bugzilla Link | [52545](https://llvm.org/bz52545) | | Version | unspecified | | OS | All | | Reporter | LLVM Bugzilla Contributor | | CC | @AaronBallman,@zygoloid | ## Extended Description We have __has_feature and __has_attribute and a number of other variants so that code can take advantage of new features while still functioning with older clang versions. We should also have __has_pragma for the same reason. In particular, I was recently bitten by trying to use the clang fp reassociate(on) pragma, which is available in clang-11+; rather than producing an "unknown pragma" warning, older clangs produce the error "invalid option 'reassociate'; expected contract". An alternative solution (which would have worked in my case) would be to have unknown pragma options always be a warning instead of an error, but in general it's a better option to have __has_pragma, because it gives the user the building block to fallback on a different solution or to produce an error in cases where that's more appropriate. I think a minimum-viable __has_pragma would take a string argument that's the entire pragma body and produce a result of 1 if the string is an accepted pragma for the current target.
pkasting commented 2 months ago

This would be useful for Chromium, as we are trying to use -Wunsafe-buffer-usage, and the current way to exclude code from its analysis is with #pragma clang unsafe-buffer-usage begin/end. Being able to test for that pragma (or determine the version of support, as proposed above) would be nice.