r367027 - Implement P1771
Keane, Erich via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 25 09:45:58 PDT 2019
Ah! Thanks. I’ll fix it now.
From: Richard Smith [mailto:richard at metafoo.co.uk]
Sent: Thursday, July 25, 2019 9:26 AM
To: Keane, Erich <erich.keane at intel.com>
Cc: cfe-commits <cfe-commits at lists.llvm.org>
Subject: Re: r367027 - Implement P1771
On Thu, 25 Jul 2019, 08:10 Erich Keane via cfe-commits, <cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>> wrote:
Author: erichkeane
Date: Thu Jul 25 08:10:56 2019
New Revision: 367027
URL: http://llvm.org/viewvc/llvm-project?rev=367027&view=rev
Log:
Implement P1771
As passed in the Cologne meeting and treated by Core as a DR,
[[nodiscard]] was applied to constructors so that they can be diagnosed
in cases where the user forgets a variable name for a type.
The intent is to enable the library to start using this on the
constructors of scope_guard/lock_guard.
Differential Revision: https://reviews.llvm.org/D64914
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
cfe/trunk/test/Preprocessor/has_attribute.cpp
cfe/trunk/www/cxx_status.html
Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=367027&r1=367026&r2=367027&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Thu Jul 25 08:10:56 2019
@@ -2335,12 +2335,19 @@ def WarnUnused : InheritableAttr {
}
def WarnUnusedResult : InheritableAttr {
- let Spellings = [CXX11<"", "nodiscard", 201603>, C2x<"", "nodiscard">,
+ let Spellings = [CXX11<"", "nodiscard", 201907>, C2x<"", "nodiscard">,
CXX11<"clang", "warn_unused_result">,
GCC<"warn_unused_result">];
let Subjects = SubjectList<[ObjCMethod, Enum, Record, FunctionLike]>;
let Args = [StringArgument<"Message", 1>];
let Documentation = [WarnUnusedResultsDocs];
+ let AdditionalMembers = [{
+ // Check whether this the C++11 nodiscard version, even in non C++11
+ // spellings.
+ bool IsCXX11NoDiscard() const {
+ return this->getSemanticSpelling() == CXX11_nodiscard;
+ }
+ }];
}
def Weak : InheritableAttr {
Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=367027&r1=367026&r2=367027&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Thu Jul 25 08:10:56 2019
@@ -1500,6 +1500,33 @@ in any resulting diagnostics.
}
error_info &foo();
void f() { foo(); } // Does not diagnose, error_info is a reference.
+
+Additionally, discarded temporaries resulting from a call to a constructor
+marked with ``[[nodiscard]]`` or a constructor of a type marked
+``[[nodiscard]]`` will also diagnose. This also applies to type conversions that
+use the annotated ``[[nodiscard]]`` constructor or result in an annotated type.
+
+.. code-block: c++
+ struct [[nodiscard]] marked_type {/*..*/ };
+ struct marked_ctor {
+ [[nodiscard]] marked_ctor();
+ marked_ctor(int);
+ };
+
+ struct S {
+ operator marked_type() const;
+ [[nodiscard]] operator int() const;
+ };
+
+ void usages() {
+ marked_type(); // diagnoses.
+ marked_ctor(); // diagnoses.
+ marked_ctor(3); // Does not diagnose, int constructor isn't marked nodiscard.
+
+ S s;
+ static_cast<marked_type>(s); // diagnoses
+ (int)s; // diagnoses
+ }
}];
}
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=367027&r1=367026&r2=367027&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Jul 25 08:10:56 2019
@@ -7430,6 +7430,12 @@ def warn_unused_container_subscript_expr
def warn_unused_call : Warning<
"ignoring return value of function declared with %0 attribute">,
InGroup<UnusedValue>;
+def warn_unused_constructor : Warning<
+ "ignoring temporary created by a constructor declared with %0 attribute">,
+ InGroup<UnusedValue>;
+def warn_unused_constructor_msg : Warning<
+ "ignoring temporary created by a constructor declared with %0 attribute: %1">,
+ InGroup<UnusedValue>;
def warn_side_effects_unevaluated_context : Warning<
"expression with side effects has no effect in an unevaluated context">,
InGroup<UnevaluatedExpression>;
Modified: cfe/trunk/lib/AST/Expr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=367027&r1=367026&r2=367027&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Thu Jul 25 08:10:56 2019
@@ -2563,13 +2563,31 @@ bool Expr::isUnusedResultAWarning(const
case CXXTemporaryObjectExprClass:
case CXXConstructExprClass: {
if (const CXXRecordDecl *Type = getType()->getAsCXXRecordDecl()) {
- if (Type->hasAttr<WarnUnusedAttr>()) {
+ const auto *WarnURAttr = Type->getAttr<WarnUnusedResultAttr>();
+ if (Type->hasAttr<WarnUnusedAttr>() ||
+ (WarnURAttr && WarnURAttr->IsCXX11NoDiscard())) {
WarnE = this;
Loc = getBeginLoc();
R1 = getSourceRange();
return true;
}
}
+
+ const auto *CE = cast<CXXConstructExpr>(this);
+ if (const CXXConstructorDecl *Ctor = CE->getConstructor()) {
+ const auto *WarnURAttr = Ctor->getAttr<WarnUnusedResultAttr>();
+ if (WarnURAttr && WarnURAttr->IsCXX11NoDiscard()) {
+ WarnE = this;
+ Loc = getBeginLoc();
+ R1 = getSourceRange();
+
+ if (unsigned NumArgs = CE->getNumArgs())
+ R2 = SourceRange(CE->getArg(0)->getBeginLoc(),
+ CE->getArg(NumArgs - 1)->getEndLoc());
+ return true;
+ }
+ }
+
return false;
}
Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=367027&r1=367026&r2=367027&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Thu Jul 25 08:10:56 2019
@@ -2831,7 +2831,8 @@ static void handleSentinelAttr(Sema &S,
static void handleWarnUnusedResult(Sema &S, Decl *D, const ParsedAttr &AL) {
if (D->getFunctionType() &&
- D->getFunctionType()->getReturnType()->isVoidType()) {
+ D->getFunctionType()->getReturnType()->isVoidType() &&
+ !isa<CXXConstructorDecl>(D)) {
S.Diag(AL.getLoc(), diag::warn_attribute_void_function_method) << AL << 0;
return;
}
Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=367027&r1=367026&r2=367027&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Thu Jul 25 08:10:56 2019
@@ -196,6 +196,25 @@ static bool DiagnoseUnusedComparison(Sem
return true;
}
+static bool DiagnoseNoDiscard(Sema &S, const WarnUnusedResultAttr *A,
+ SourceLocation Loc, SourceRange R1,
+ SourceRange R2, bool IsCtor) {
+ if (!A)
+ return false;
+ StringRef Msg = A->getMessage();
+
+ if (Msg.empty()) {
+ if (IsCtor)
+ return S.Diag(Loc, diag::warn_unused_constructor) << A << R1 << R2;
+ return S.Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
+ }
+
+ if (IsCtor)
+ return S.Diag(Loc, diag::warn_unused_constructor_msg) << A << Msg << R1
+ << R2;
+ return S.Diag(Loc, diag::warn_unused_result_msg) << A << Msg << R1 << R2;
+}
+
void Sema::DiagnoseUnusedExprResult(const Stmt *S) {
if (const LabelStmt *Label = dyn_cast_or_null<LabelStmt>(S))
return DiagnoseUnusedExprResult(Label->getSubStmt());
@@ -254,19 +273,19 @@ void Sema::DiagnoseUnusedExprResult(cons
return;
E = WarnExpr;
+ if (const auto *Cast = dyn_cast<CastExpr>(E))
+ if (Cast->getCastKind() == CK_NoOp ||
+ Cast->getCastKind() == CK_ConstructorConversion)
+ E = Cast->getSubExpr()->IgnoreImpCasts();
+
if (const CallExpr *CE = dyn_cast<CallExpr>(E)) {
if (E->getType()->isVoidType())
return;
- if (const auto *A = cast_or_null<WarnUnusedResultAttr>(
- CE->getUnusedResultAttr(Context))) {
- StringRef Msg = A->getMessage();
- if (!Msg.empty())
- Diag(Loc, diag::warn_unused_result_msg) << A << Msg << R1 << R2;
- else
- Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
+ if (DiagnoseNoDiscard(*this, cast_or_null<WarnUnusedResultAttr>(
+ CE->getUnusedResultAttr(Context)),
+ Loc, R1, R2, /*isCtor=*/false))
return;
- }
// If the callee has attribute pure, const, or warn_unused_result, warn with
// a more specific message to make it clear what is happening. If the call
@@ -284,9 +303,24 @@ void Sema::DiagnoseUnusedExprResult(cons
return;
}
}
+ } else if (const auto *CE = dyn_cast<CXXConstructExpr>(E)) {
+ if (const CXXConstructorDecl *Ctor = CE->getConstructor()) {
+ const auto *A = Ctor->getAttr<WarnUnusedResultAttr>();
+ A = A ? A : Ctor->getParent()->getAttr<WarnUnusedResultAttr>();
+ if (DiagnoseNoDiscard(*this, A, Loc, R1, R2, /*isCtor=*/true))
+ return;
+ }
+ } else if (const auto *ILE = dyn_cast<InitListExpr>(E)) {
+ if (const TagDecl *TD = ILE->getType()->getAsTagDecl()) {
+
+ if (DiagnoseNoDiscard(*this, TD->getAttr<WarnUnusedResultAttr>(), Loc, R1,
+ R2, /*isCtor=*/false))
+ return;
+ }
} else if (ShouldSuppress)
return;
+ E = WarnExpr;
if (const ObjCMessageExpr *ME = dyn_cast<ObjCMessageExpr>(E)) {
if (getLangOpts().ObjCAutoRefCount && ME->isDelegateInitCall()) {
Diag(Loc, diag::err_arc_unused_init_message) << R1;
@@ -294,14 +328,9 @@ void Sema::DiagnoseUnusedExprResult(cons
}
const ObjCMethodDecl *MD = ME->getMethodDecl();
if (MD) {
- if (const auto *A = MD->getAttr<WarnUnusedResultAttr>()) {
- StringRef Msg = A->getMessage();
- if (!Msg.empty())
- Diag(Loc, diag::warn_unused_result_msg) << A << Msg << R1 << R2;
- else
- Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
+ if (DiagnoseNoDiscard(*this, MD->getAttr<WarnUnusedResultAttr>(), Loc, R1,
+ R2, /*isCtor=*/false))
return;
- }
}
} else if (const PseudoObjectExpr *POE = dyn_cast<PseudoObjectExpr>(E)) {
const Expr *Source = POE->getSyntacticForm();
Modified: cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp?rev=367027&r1=367026&r2=367027&view=diff
==============================================================================
--- cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp (original)
+++ cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp Thu Jul 25 08:10:56 2019
@@ -80,6 +80,50 @@ void cxx2a_use() {
conflicting_reason(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute: special reason}}
}
+namespace p1771 {
+struct[[nodiscard("Don't throw me away!")]] ConvertTo{};
+struct S {
+ [[nodiscard]] S();
+ [[nodiscard("Don't let that S-Char go!")]] S(char);
+ S(int);
+ [[gnu::warn_unused_result]] S(double);
+ operator ConvertTo();
+ [[nodiscard]] operator int();
+ [[nodiscard("Don't throw away as a double")]] operator double();
+};
+
+struct[[nodiscard("Don't throw me away either!")]] Y{};
+
+void usage() {
+ S(); // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute}}
+ S('A'); // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute: Don't let that S-Char go!}}
+ S(1);
+ S(2.2);
+ Y(); // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute: Don't throw me away either!}}
+ S s;
+ ConvertTo{}; // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute: Don't throw me away!}}
+
+// AST is different in C++20 mode, pre-2017 a move ctor for ConvertTo is there
+// as well, hense the constructor warning.
+#if __cplusplus >= 201703L
+// expected-warning at +4 {{ignoring return value of function declared with 'nodiscard' attribute: Don't throw me away!}}
+#else
+// expected-warning at +2 {{ignoring temporary created by a constructor declared with 'nodiscard' attribute: Don't throw me away!}}
+#endif
+ (ConvertTo) s;
+ (int)s; // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ (S)'c'; // expected-warning {{ignoring temporary created by a constructor declared with 'nodiscard' attribute: Don't let that S-Char go!}}
+#if __cplusplus >= 201703L
+// expected-warning at +4 {{ignoring return value of function declared with 'nodiscard' attribute: Don't throw me away!}}
+#else
+// expected-warning at +2 {{ignoring temporary created by a constructor declared with 'nodiscard' attribute: Don't throw me away!}}
+#endif
+ static_cast<ConvertTo>(s);
+ static_cast<int>(s); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+ static_cast<double>(s); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute: Don't throw away as a double}}
+}
+}; // namespace p1771
+
#ifdef EXT
// expected-warning at 5 {{use of the 'nodiscard' attribute is a C++17 extension}}
// expected-warning at 9 {{use of the 'nodiscard' attribute is a C++17 extension}}
@@ -91,4 +135,10 @@ void cxx2a_use() {
// expected-warning at 71 {{use of the 'nodiscard' attribute is a C++2a extension}}
// expected-warning at 73 {{use of the 'nodiscard' attribute is a C++2a extension}}
// expected-warning at 74 {{use of the 'nodiscard' attribute is a C++2a extension}}
+// expected-warning at 84 {{use of the 'nodiscard' attribute is a C++2a extension}}
+// expected-warning at 86 {{use of the 'nodiscard' attribute is a C++17 extension}}
+// expected-warning at 87 {{use of the 'nodiscard' attribute is a C++2a extension}}
+// expected-warning at 91 {{use of the 'nodiscard' attribute is a C++17 extension}}
+// expected-warning at 92 {{use of the 'nodiscard' attribute is a C++2a extension}}
+// expected-warning at 95 {{use of the 'nodiscard' attribute is a C++2a extension}}
#endif
Modified: cfe/trunk/test/Preprocessor/has_attribute.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/has_attribute.cpp?rev=367027&r1=367026&r2=367027&view=diff
==============================================================================
--- cfe/trunk/test/Preprocessor/has_attribute.cpp (original)
+++ cfe/trunk/test/Preprocessor/has_attribute.cpp Thu Jul 25 08:10:56 2019
@@ -63,7 +63,7 @@ CXX11(unlikely)
// CHECK: maybe_unused: 201603L
// ITANIUM: no_unique_address: 201803L
// WINDOWS: no_unique_address: 0
-// CHECK: nodiscard: 201603L
+// CHECK: nodiscard: 201907L
// CHECK: noreturn: 200809L
// FIXME(201803L) CHECK: unlikely: 0
Modified: cfe/trunk/www/cxx_status.html
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/www/cxx_status.html?rev=367027&r1=367026&r2=367027&view=diff
==============================================================================
--- cfe/trunk/www/cxx_status.html (original)
+++ cfe/trunk/www/cxx_status.html Thu Jul 25 08:10:56 2019
@@ -664,7 +664,7 @@ version 3.7.
</tr>
<tr> <!-- from Cologne 2019 -->
<td><a href="http://wg21.link/p1771r1">P1771R1</a> (<a href="#dr">DR</a>)</td>
- <td class="none" align="center">No</td>
+ <td class="none" align="center">SVN</td>
This should be class="svn"
</tr>
<tr>
<td><tt>[[maybe_unused]]</tt> attribute</td>
_______________________________________________
cfe-commits mailing list
cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190725/a3743cb8/attachment-0001.html>
More information about the cfe-commits
mailing list