[clang] 2edb89c - Lex arguments for __has_cpp_attribute and friends as expanded tokens
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 18 13:13:06 PDT 2021
On Mon, 18 Oct 2021 at 12:56, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Mon, Oct 18, 2021 at 3:52 PM Richard Smith <richard at metafoo.co.uk>
> wrote:
> >
> > On Mon, 18 Oct 2021 at 12:48, Aaron Ballman <aaron at aaronballman.com>
> wrote:
> >>
> >> On Mon, Oct 18, 2021 at 3:33 PM Richard Smith <richard at metafoo.co.uk>
> wrote:
> >> >
> >> > On Sun, 17 Oct 2021 at 04:58, Aaron Ballman via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
> >> >>
> >> >>
> >> >> Author: Aaron Ballman
> >> >> Date: 2021-10-17T07:54:48-04:00
> >> >> New Revision: 2edb89c746848c52964537268bf03e7906bf2542
> >> >>
> >> >> URL:
> https://github.com/llvm/llvm-project/commit/2edb89c746848c52964537268bf03e7906bf2542
> >> >> DIFF:
> https://github.com/llvm/llvm-project/commit/2edb89c746848c52964537268bf03e7906bf2542.diff
> >> >>
> >> >> LOG: Lex arguments for __has_cpp_attribute and friends as expanded
> tokens
> >> >>
> >> >> The C and C++ standards require the argument to __has_cpp_attribute
> and
> >> >> __has_c_attribute to be expanded ([cpp.cond]p5). It would make
> little sense
> >> >> to expand the argument to those operators but not expand the
> argument to
> >> >> __has_attribute and __has_declspec, so those were both also changed
> in this
> >> >> patch.
> >> >>
> >> >> Note that it might make sense for the other builtins to also expand
> their
> >> >> argument, but it wasn't as clear to me whether the behavior would be
> correct
> >> >> there, and so they were left for a future revision.
> >> >>
> >> >> Added:
> >> >> clang/test/Preprocessor/has_attribute_errors.cpp
> >> >>
> >> >> Modified:
> >> >> clang/docs/ReleaseNotes.rst
> >> >> clang/lib/Lex/PPMacroExpansion.cpp
> >> >> clang/test/Preprocessor/has_attribute.c
> >> >> clang/test/Preprocessor/has_attribute.cpp
> >> >> clang/test/Preprocessor/has_c_attribute.c
> >> >>
> >> >> Removed:
> >> >>
> >> >>
> >> >>
> >> >>
> ################################################################################
> >> >> diff --git a/clang/docs/ReleaseNotes.rst
> b/clang/docs/ReleaseNotes.rst
> >> >> index 6501a4870e2a6..263eae83036df 100644
> >> >> --- a/clang/docs/ReleaseNotes.rst
> >> >> +++ b/clang/docs/ReleaseNotes.rst
> >> >> @@ -110,6 +110,13 @@ Attribute Changes in Clang
> >> >> attribute is handled instead, e.g. in ``handleDeclAttribute``.
> >> >> (This was changed in order to better support attributes in code
> completion).
> >> >>
> >> >> +- __has_cpp_attribute, __has_c_attribute, __has_attribute, and
> __has_declspec
> >> >> + will now macro expand their argument. This causes a change in
> behavior for
> >> >> + code using ``__has_cpp_attribute(__clang__::attr)`` (and same for
> >> >> + ``__has_c_attribute``) where it would previously expand to ``0``
> for all
> >> >> + attributes, but will now issue an error due to the expansion of
> the
> >> >> + predefined ``__clang__`` macro.
> >> >> +
> >> >> Windows Support
> >> >> ---------------
> >> >>
> >> >>
> >> >> diff --git a/clang/lib/Lex/PPMacroExpansion.cpp
> b/clang/lib/Lex/PPMacroExpansion.cpp
> >> >> index bf19f538647e6..5a0fa5184e38b 100644
> >> >> --- a/clang/lib/Lex/PPMacroExpansion.cpp
> >> >> +++ b/clang/lib/Lex/PPMacroExpansion.cpp
> >> >> @@ -1293,7 +1293,7 @@ static bool EvaluateHasIncludeNext(Token &Tok,
> >> >> /// integer values.
> >> >> static void
> EvaluateFeatureLikeBuiltinMacro(llvm::raw_svector_ostream& OS,
> >> >> Token &Tok,
> IdentifierInfo *II,
> >> >> - Preprocessor &PP,
> >> >> + Preprocessor &PP, bool
> ExpandArgs,
> >> >> llvm::function_ref<
> >> >> int(Token &Tok,
> >> >> bool
> &HasLexedNextTok)> Op) {
> >> >> @@ -1319,7 +1319,10 @@ static void
> EvaluateFeatureLikeBuiltinMacro(llvm::raw_svector_ostream& OS,
> >> >> bool SuppressDiagnostic = false;
> >> >> while (true) {
> >> >> // Parse next token.
> >> >> - PP.LexUnexpandedToken(Tok);
> >> >> + if (ExpandArgs)
> >> >> + PP.Lex(Tok);
> >> >> + else
> >> >> + PP.LexUnexpandedToken(Tok);
> >> >
> >> >
> >> > How does this handle things like:
> >> >
> >> > #define RPAREN )
> >> > #if __has_attribute(clang::fallthrough RPAREN
> >> >
> >> > ? I think that should be an error: the ) token should not be produced
> by macro expansion, analogous to the behavior of function-like macros. But
> I imagine unless we're careful here, we'll allow that macro expansion to
> terminate the "macro".
> >>
> >> I agree, I think that should be an error. We handle it reasonably too;
> I get:
> >>
> >> error: missing ')' after 'clang'
> >>
> >> though it appears there is some compiler divergence here:
> >> https://godbolt.org/z/c84cb3PW7
> >
> >
> > Oops, I meant __has_cpp_attribute, not __has_attribute. We mishandle
> that one just like GCC: https://godbolt.org/z/Ej84Kca16
Digging a bit more: https://godbolt.org/z/4Yo578b8n
__has_cpp_attribute(clang::FT RPAREN and __has_attribute(FT RPAREN
are incorrectly allowed, but __has_cpp_attribute(FT RPAREN is rejected.
Well that's just neat. I was trying to avoid lexing an unexpanded
> token and then expanding it later if it was an identifier, but it
> sounds like I should be taking that approach here. (Is that even
> possible to do?)
>
If you want to exactly match macro argument handling, you'll need to first
lex unexpanded tokens until you find the `)`, and then expand them
separately. Instead, you could ask if the ) token came from macro
expansion, and if so, reject, but that will still mishandle cases like
#define M1 M2(
#define M2() fallthrough
__has_cpp_attribute( M1 ) )
Perhaps we can move the interception of builtin function-like macros to
after we prepare to do macro expansion, so we can reuse the existing code
for gathering the arguments?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211018/abf32995/attachment-0001.html>
More information about the cfe-commits
mailing list