aaron.ballman added inline comments.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:2833-2835
if (D->getFunctionType() &&
- D->getFunctionType()->getReturnType()->isVoidType()) {
+ D->getFunctionType()->getReturnType()->isVoidType() &&
+ !isa<CXXConstructorDecl>(D)) {
----------------
erichkeane wrote:
> aaron.ballman wrote:
> > erichkeane wrote:
> > > aaron.ballman wrote:
> > > > I'm not certain about this -- we do not allow you to put
> > > > `__attribute__((warn_unused_result))` on a constructor declaration, and
> > > > neither does GCC. Do you know if they're planning to change their
> > > > behavior as well? https://godbolt.org/z/kDZu3Q
> > > I'm unsure about GCC's intent, though they will likely be changing the
> > > [[nodiscard]] behavior due to the vote (and I doubt they will have an "if
> > > C++20" test either.
> > >
> > > Do we have a good way to exclude the alternate spellings?
> > > isCXX11Attribute only checks syntax, not the actual spelling. ParsedAttr
> > > doesn't seem to even contain a way to get which namespace it is
> > > associated with either. Is something like that valuable to add?
> > The way we currently do this is `(isCXX11Attribute() || isC2xAttribute())
> > && !getScopeName()`, but I am not opposed to going with an additional
> > member. Do you have a preference either way?
> An additional member of Spelling-Kind (vs Syntax kind) seems very valuable
> here, though I wonder if it is valuable enough of a change for TableGen. The
> isCXX11Attribute && !getScopeName seems a little obtuse from a readers
> perspective.
Yeah, I would love to solve this through the tablegen interface at some point,
as that solves other problems we have (like the fact that we don't AST dump
with the syntax the user wrote). Let's hold off on that for now and use the
obtuse approach, but with a FIXME for us to come back and touch it again once
we get tablegen to track more syntactic information along with the semantic
attribute object.
================
Comment at: clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp:87-88
// expected-warning@28 {{use of the 'nodiscard' attribute is a C++17
extension}}
+// expected-warning@66 {{use of the 'nodiscard' attribute is a C++17
extension}}
+// expected-warning@71 {{use of the 'nodiscard' attribute is a C++17
extension}}
#endif
----------------
erichkeane wrote:
> aaron.ballman wrote:
> > Is Core treating this paper as a DR? I don't have a strong opinion here,
> > but for the nodiscard with a message version, I made it a C++2a extension.
> > I don't have a strong opinion, but I sort of prefer doing whatever Core
> > decides.
> I am unfamiliar with what Core is treating it as, but my understanding is
> that EWG encouraged implementers to treat it as such.
We expose the attribute in all its glory in all language modes, so these
changes already do what we want in effect. The only real question is whether we
want to claim it's a C++17 extension or a C++2a extension. If a user turns on
extension warnings, we should probably tell them when the feature was added,
which is C++2a. It would be a bit weird to claim this is a C++17 when the
feature test for it is `__has_attribute(nodiscard) == 201907L` (due to the
normative wording changes).
But if Core moves it as a DR, then C++17 is fine, though I suppose SD-6 would
need to make clear what is required for each given attribute feature test value
to give us the answer.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64914/new/
https://reviews.llvm.org/D64914
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
