[PATCH] D64914: Implement P1771

[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 (or at least understand better where the inconsistencies are).

This is also missing documentation changes and the updated feature test macro value.



================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:2833-2835
   if (D->getFunctionType() &&
-      D->getFunctionType()->getReturnType()->isVoidType()) {
+      D->getFunctionType()->getReturnType()->isVoidType() &&
+      !isa<CXXConstructorDecl>(D)) {
----------------
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


================
Comment at: clang/lib/Sema/SemaStmt.cpp:284
+    if (const CXXConstructorDecl *Ctor = CE->getConstructor()) {
+      if (const Attr *A = Ctor->getAttr<WarnUnusedResultAttr>()) {
+        Diag(Loc, diag::warn_unused_constructor) << A << R1 << R2;
----------------
`const auto *` here and below?


================
Comment at: clang/lib/Sema/SemaStmt.cpp:288-295
+      if (const Attr *A = Ctor->getAttr<PureAttr>()) {
+        Diag(Loc, diag::warn_unused_constructor) << A << R1 << R2;
+        return;
+      }
+      if (const Attr *A = Ctor->getAttr<ConstAttr>()) {
+        Diag(Loc, diag::warn_unused_constructor) << A << R1 << R2;
+        return;
----------------
GCC does not warn on these cases and does not let you write the attributes on a constructor. Do you know if they're intending to implement this?


================
Comment at: clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp:70
+  };
+  struct [[nodiscard]]Y{};
+  void usage() {
----------------
Formatting is a bit messy here.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64914/new/

https://reviews.llvm.org/D64914