mboehme added a comment.
In D126061#3590896 <https://reviews.llvm.org/D126061#3590896>, @dyung wrote:
> @mboehme, one of our internal tests started to fail to compile after this
> change due to the compiler no longer accepting what I think should be a valid
> attribute declaration. I have filed issue #56077 for the problem, can you
> take a look?
Attributes are not allowed to occur in this position -- I'll discuss in more
detail on #56077.
See also the response I'm making on a code comment, where a reviewer pointed
out to me that the C++ standard does not allow `[[]]` attributes before `extern
"C"`.
================
Comment at: clang/lib/Parse/Parser.cpp:1164
DS.getParsedSpecifiers() == DeclSpec::PQ_StorageClassSpecifier) {
- Decl *TheDecl = ParseLinkage(DS, DeclaratorContext::File);
+ Decl *TheDecl = ParseLinkage(DS, DeclaratorContext::File, Attrs);
return Actions.ConvertDeclToDeclGroup(TheDecl);
----------------
mboehme wrote:
> aaron.ballman wrote:
> > rsmith wrote:
> > > We should `ProhibitAttrs` here rather than passing them on.
> > > ```
> > > [[]] extern "C" void f();
> > > ```
> > > ... is invalid. (Per the grammar in
> > > https://eel.is/c++draft/dcl.dcl#dcl.pre-1 and
> > > https://eel.is/c++draft/dcl.dcl#dcl.link-2 an attribute-specifier-seq
> > > can't appear here.)
> > +1, looks like we're missing test coverage for that case (but those
> > diagnostics by GCC or MSVC... hoo boy!): https://godbolt.org/z/cTfPbK837
> Fixed and added a test in test/Parser/cxx0x-attributes.cpp.
Update: @dyung is seeing errors on their codebase because of this -- see also
the comment they added to this patch.
It's not unexpected that people would have this incorrect usage in their
codebases because we used to allow it. At the same time, it's not hard to fix,
and I would generally expect this kind of error to occur in first-party code
(that is easily fixed) rather than third-party libraries, because the latter
usually compile on GCC, and GCC disallows this usage.
Still, I wanted to discuss whether you think we need to reinstate support for
this (incorrect) usage temporarily, with a deprecation warning rather than an
error?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126061/new/
https://reviews.llvm.org/D126061
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
