-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
[preprocessor] Clang doesn't warn about preprocessing directives in function-like macro arguments #73598
Comments
@llvm/issue-subscribers-clang-frontend Author: Stephan T. Lavavej (StephanTLavavej)
[N4964](https://isocpp.org/files/papers/N4964.pdf) \[cpp.replace.general\]/13:
> The sequence of preprocessing tokens bounded by the outside-most matching parentheses forms the list of arguments for the function-like macro. The individual arguments within the list are separated by comma preprocessing tokens, but comma preprocessing tokens between matching inner parentheses do not separate arguments. If there are sequences of preprocessing tokens within the list of arguments that would otherwise act as preprocessing directives, the behavior is undefined. When preprocessing directives are embedded in function-like macro arguments, MSVC appears to reject with its traditional (horribly deficient) preprocessor, and emits a dedicated warning with its modern (more conformant) preprocessor (and then later rejects, at least some of the time). Clang appears to silently accept, which is surprising.
#include <cstdio>
#define OUTER_MACRO(ARG) std::puts(ARG)
int main() {
OUTER_MACRO("I have "
#ifdef MANY
"1729"
#else
"5"
#endif
" cute fluffy kittens.");
}
This allowed UB to accumulate in libc++'s test suite, see #73440. |
This recently came up in WG21 as part of P2843 and cplusplus/papers#1413 The undefined behavior in the spec is how historically C allowed implementation divergence. I think we should wait for WG21 to decide whether specific directives should be allowed/removed. If my recall is correct, clang is consistent with GCC. Adding a warning now might lead to too much churn and noise. I struggle to find the minutes of that specific discussion though. |
@cor3ntin : all of the discussion on that paper happened in "SG12", so we put it on that wiki: https://wiki.edg.com/bin/view/Wg21kona2023/SG12 |
Found while running libc++'s tests with MSVC's STL. * Avoid MSVC warning C5101: use of preprocessor directive in function-like macro argument list is undefined behavior. + We can easily make this portable by extracting `const bool is_newlib`. + Followup to #73440. + See #73598. + See #73836. * Avoid MSVC warning C4267: 'return': conversion from 'size_t' to 'int', possible loss of data. + This warning is valid, but harmless for the test, so `static_cast<int>` will avoid it. * Avoid MSVC warning C4146: unary minus operator applied to unsigned type, result still unsigned. + This warning is also valid (the scenario is sometimes intentional, but surprising enough that it's worth warning about). This is a C++17 test, so we can easily avoid it by testing `is_signed_v` at compile-time before testing `m < 0` and `n < 0` at run-time. * Silence MSVC warning C4310: cast truncates constant value. + These warnings are being emitted by `T(255)`. Disabling the warning is simpler than attempting to restructure the code. + Followup to #79791. * MSVC no longer emits warning C4521: multiple copy constructors specified. + This warning was removed from the compiler, since at least 2021-12-09.
Found while running libc++'s tests with MSVC's STL. * Avoid MSVC warning C5101: use of preprocessor directive in function-like macro argument list is undefined behavior. + We can easily make this portable by extracting `const bool is_newlib`. + Followup to llvm#73440. + See llvm#73598. + See llvm#73836. * Avoid MSVC warning C4267: 'return': conversion from 'size_t' to 'int', possible loss of data. + This warning is valid, but harmless for the test, so `static_cast<int>` will avoid it. * Avoid MSVC warning C4146: unary minus operator applied to unsigned type, result still unsigned. + This warning is also valid (the scenario is sometimes intentional, but surprising enough that it's worth warning about). This is a C++17 test, so we can easily avoid it by testing `is_signed_v` at compile-time before testing `m < 0` and `n < 0` at run-time. * Silence MSVC warning C4310: cast truncates constant value. + These warnings are being emitted by `T(255)`. Disabling the warning is simpler than attempting to restructure the code. + Followup to llvm#79791. * MSVC no longer emits warning C4521: multiple copy constructors specified. + This warning was removed from the compiler, since at least 2021-12-09.
N4964 [cpp.replace.general]/13:
When preprocessing directives are embedded in function-like macro arguments, MSVC appears to reject with its traditional (horribly deficient) preprocessor, and emits a dedicated warning with its modern (more conformant) preprocessor (and then later rejects, at least some of the time). Clang appears to silently accept, which is surprising.
This allowed UB to accumulate in libc++'s test suite, see #73440.
The text was updated successfully, but these errors were encountered: