[PATCH] D64914: Implement P1771

2019-07-18 Thread Erich Keane via Phabricator via cfe-commits
erichkeane created this revision. erichkeane added reviewers: aaron.ballman, chandlerc. Herald added a project: clang. EWG met on 7/18/19 and came to the conclusion that our initial intent was to apply to constructors, so approved the paper and expected implementers to use implementers discretion

[PATCH] D64914: Implement P1771

2019-07-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. There seems to be some separable concerns in this patch. I'd rather real with `warn_unused_result`, `const`, and `pure` attributes separately only because those are under the `gnu` attribute namespace and I'd prefer to match GNU's semantics for those attributes (o

[PATCH] D64914: Implement P1771

2019-07-18 Thread Erich Keane via Phabricator via cfe-commits
erichkeane marked 5 inline comments as done. erichkeane added a comment. In D64914#1591744 , @aaron.ballman wrote: > There seems to be some separable concerns in this patch. I'd rather real with > `warn_unused_result`, `const`, and `pure` attributes sepa

[PATCH] D64914: Implement P1771

2019-07-19 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 210753. erichkeane marked an inline comment as done. erichkeane added a comment. Removes pure/const, limits change to just CXX11 spelling, and changes docs/FeatureTestMacro. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64914/new/ https://review

[PATCH] D64914: Implement P1771

2019-07-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2346 +// spellings. +bool IsCXX11NoDiscard() const { + return this->getSemanticSpelling() == CXX11_nodiscard; I don't think this is strictly required, but perhaps it's

[PATCH] D64914: Implement P1771

2019-07-19 Thread Erich Keane via Phabricator via cfe-commits
erichkeane marked 2 inline comments as done. erichkeane added a comment. I'll need to rebase this on another patch soon anyway, so I'll hold off until next week to update this particularly since we have some open questions. The additional TableGen work is tempting to do, though I'm not complete

[PATCH] D64914: Implement P1771

2019-07-19 Thread Aaron Ballman via Phabricator via cfe-commits
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(D)) {

[PATCH] D64914: Implement P1771

2019-07-21 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. Please update cxx_status.html to mark P1771R1 as implemented in SVN. 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 {{

[PATCH] D64914: Implement P1771

2019-07-22 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 211094. erichkeane marked an inline comment as done. erichkeane added a comment. Rebased and did all the comments (including the www_status). @aaron.ballman : Wasn't positive what you meant about the conversion functions, but I think I got one? I would

[PATCH] D64914: Implement P1771

2019-07-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D64914#1595660 , @erichkeane wrote: > Rebased and did all the comments (including the www_status). @aaron.ballman > : Wasn't positive what you meant about the conversion functions, but I think > I got one? I was talki

[PATCH] D64914: Implement P1771

2019-07-23 Thread Erich Keane via Phabricator via cfe-commits
erichkeane marked 2 inline comments as done. erichkeane added a comment. Ugg... well conversion functions are an interesting bit, as well as Type{}; with no constructors. It ends up being a function style cast to an init-list-expr, so its going to require a bit more work. Stay tuned :)

[PATCH] D64914: Implement P1771

2019-07-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:2835 + D->getFunctionType()->getReturnType()->isVoidType() && + !isa(D)) { S.Diag(AL.getLoc(), diag::warn_attribute_void_function_method

[PATCH] D64914: Implement P1771

2019-07-24 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 211482. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64914/new/ https://reviews.llvm.org/D64914 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/AST/

[PATCH] D64914: Implement P1771

2019-07-24 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. Disregard that last commit... i ended up breaking stuff apparently. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64914/new/ https://reviews.llvm.org/D64914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D64914: Implement P1771

2019-07-24 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 211504. erichkeane added a comment. Casting implementation had to happen in SemaStmt, because other 'warn unused result' stuff depends on them being the way they are. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64914/new/ https://reviews.llvm.

[PATCH] D64914: Implement P1771

2019-07-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:296-301 + if (A) { +StringRef Msg = A->getMessage(); +if (!Msg.empty()) + Diag(Loc, diag::warn_unused_constructor_msg) << A << Msg << R1 << R2; +else + Diag

[PATCH] D64914: Implement P1771

2019-07-25 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 211740. erichkeane marked 2 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64914/new/ https://reviews.llvm.org/D64914 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/include/clang/

[PATCH] D64914: Implement P1771

2019-07-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a minor naming nit. Comment at: clang/lib/Sema/SemaStmt.cpp:201 + SourceLocation Loc, SourceRange R1, +

[PATCH] D64914: Implement P1771

2019-07-25 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL367027: Implement P1771 (authored by erichkeane, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D64914?vs=211740