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.

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

Open
StephanTLavavej opened this issue Nov 28, 2023 · 4 comments
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer clang:frontend Language frontend issues, e.g. anything involving "Sema"

Comments

@StephanTLavavej
Copy link
Member

N4964 [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.

C:\Temp>type meow.cpp
#include <cstdio>

#define OUTER_MACRO(ARG) std::puts(ARG)

int main() {
    OUTER_MACRO("I have "
#ifdef MANY
                "1729"
#else
                 "5"
#endif
                " cute fluffy kittens.");
}
C:\Temp>cl /EHsc /nologo /W4 meow.cpp && meow
meow.cpp
meow.cpp(12): error C2121: '#': invalid character: possibly the result of a macro expansion
meow.cpp(6): error C2146: syntax error: missing ')' before identifier 'ifdef'
meow.cpp(6): error C2146: syntax error: missing ';' before identifier 'ifdef'
meow.cpp(6): error C2065: 'ifdef': undeclared identifier
meow.cpp(6): error C2146: syntax error: missing ';' before identifier 'MANY'
meow.cpp(6): error C2065: 'MANY': undeclared identifier
meow.cpp(6): error C2143: syntax error: missing ';' before 'string'
meow.cpp(6): error C2143: syntax error: missing ';' before 'else'
meow.cpp(12): error C2181: illegal else without matching if
meow.cpp(6): error C2146: syntax error: missing ';' before identifier 'endif'
meow.cpp(6): error C2065: 'endif': undeclared identifier
meow.cpp(6): error C2059: syntax error: ')'

C:\Temp>cl /EHsc /nologo /W4 /Zc:preprocessor meow.cpp && meow
meow.cpp
meow.cpp(7): warning C5101: use of preprocessor directive in function-like macro argument list is undefined behavior
meow.cpp(9): warning C5101: use of preprocessor directive in function-like macro argument list is undefined behavior
meow.cpp(11): warning C5101: use of preprocessor directive in function-like macro argument list is undefined behavior
meow.cpp(6): error C2143: syntax error: missing ')' before '#'
meow.cpp(6): error C2143: syntax error: missing ';' before '#'
meow.cpp(6): error C2059: syntax error: '#'
meow.cpp(6): error C2059: syntax error: ')'

C:\Temp>clang-cl /EHsc /nologo /W4 meow.cpp && meow
I have 5 cute fluffy kittens.

C:\Temp>clang-cl -v
clang version 16.0.5
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\Llvm\x64\bin

This allowed UB to accumulate in libc++'s test suite, see #73440.

@github-actions github-actions bot added the clang Clang issues not falling into any other category label Nov 28, 2023
@EugeneZelenko EugeneZelenko added clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer and removed clang Clang issues not falling into any other category labels Nov 28, 2023
@shafik
Copy link
Collaborator

shafik commented Nov 28, 2023

CC @AaronBallman @cor3ntin

@shafik shafik added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Nov 28, 2023
@llvmbot
Copy link
Member

llvmbot commented Nov 28, 2023

@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.

C:\Temp&gt;type meow.cpp
#include &lt;cstdio&gt;

#define OUTER_MACRO(ARG) std::puts(ARG)

int main() {
    OUTER_MACRO("I have "
#ifdef MANY
                "1729"
#else
                 "5"
#endif
                " cute fluffy kittens.");
}
C:\Temp&gt;cl /EHsc /nologo /W4 meow.cpp &amp;&amp; meow
meow.cpp
meow.cpp(12): error C2121: '#': invalid character: possibly the result of a macro expansion
meow.cpp(6): error C2146: syntax error: missing ')' before identifier 'ifdef'
meow.cpp(6): error C2146: syntax error: missing ';' before identifier 'ifdef'
meow.cpp(6): error C2065: 'ifdef': undeclared identifier
meow.cpp(6): error C2146: syntax error: missing ';' before identifier 'MANY'
meow.cpp(6): error C2065: 'MANY': undeclared identifier
meow.cpp(6): error C2143: syntax error: missing ';' before 'string'
meow.cpp(6): error C2143: syntax error: missing ';' before 'else'
meow.cpp(12): error C2181: illegal else without matching if
meow.cpp(6): error C2146: syntax error: missing ';' before identifier 'endif'
meow.cpp(6): error C2065: 'endif': undeclared identifier
meow.cpp(6): error C2059: syntax error: ')'

C:\Temp&gt;cl /EHsc /nologo /W4 /Zc:preprocessor meow.cpp &amp;&amp; meow
meow.cpp
meow.cpp(7): warning C5101: use of preprocessor directive in function-like macro argument list is undefined behavior
meow.cpp(9): warning C5101: use of preprocessor directive in function-like macro argument list is undefined behavior
meow.cpp(11): warning C5101: use of preprocessor directive in function-like macro argument list is undefined behavior
meow.cpp(6): error C2143: syntax error: missing ')' before '#'
meow.cpp(6): error C2143: syntax error: missing ';' before '#'
meow.cpp(6): error C2059: syntax error: '#'
meow.cpp(6): error C2059: syntax error: ')'

C:\Temp&gt;clang-cl /EHsc /nologo /W4 meow.cpp &amp;&amp; meow
I have 5 cute fluffy kittens.

C:\Temp&gt;clang-cl -v
clang version 16.0.5
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\Llvm\x64\bin

This allowed UB to accumulate in libc++'s test suite, see #73440.

@cor3ntin
Copy link
Contributor

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.

@erichkeane

@erichkeane
Copy link
Collaborator

@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

ldionne pushed a commit that referenced this issue May 28, 2024
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.
vg0204 pushed a commit to vg0204/llvm-project that referenced this issue May 29, 2024
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer clang:frontend Language frontend issues, e.g. anything involving "Sema"
Projects
None yet
Development

No branches or pull requests

6 participants