On Mon, 18 Oct 2021 at 12:56, Aaron Ballman <aa...@aaronballman.com> wrote:
> On Mon, Oct 18, 2021 at 3:52 PM Richard Smith <rich...@metafoo.co.uk> > wrote: > > > > On Mon, 18 Oct 2021 at 12:48, Aaron Ballman <aa...@aaronballman.com> > wrote: > >> > >> On Mon, Oct 18, 2021 at 3:33 PM Richard Smith <rich...@metafoo.co.uk> > wrote: > >> > > >> > On Sun, 17 Oct 2021 at 04:58, Aaron Ballman via cfe-commits < > cfe-commits@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?
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
