On Thu, 25 Jul 2019, 08:10 Erich Keane via cfe-commits, < cfe-commits@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@+4 {{ignoring return value of function declared with > 'nodiscard' attribute: Don't throw me away!}} > +#else > +// expected-warning@+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@+4 {{ignoring return value of function declared with > 'nodiscard' attribute: Don't throw me away!}} > +#else > +// expected-warning@+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@5 {{use of the 'nodiscard' attribute is a C++17 > extension}} > // expected-warning@9 {{use of the 'nodiscard' attribute is a C++17 > extension}} > @@ -91,4 +135,10 @@ void cxx2a_use() { > // expected-warning@71 {{use of the 'nodiscard' attribute is a C++2a > extension}} > // expected-warning@73 {{use of the 'nodiscard' attribute is a C++2a > extension}} > // expected-warning@74 {{use of the 'nodiscard' attribute is a C++2a > extension}} > +// expected-warning@84 {{use of the 'nodiscard' attribute is a C++2a > extension}} > +// expected-warning@86 {{use of the 'nodiscard' attribute is a C++17 > extension}} > +// expected-warning@87 {{use of the 'nodiscard' attribute is a C++2a > extension}} > +// expected-warning@91 {{use of the 'nodiscard' attribute is a C++17 > extension}} > +// expected-warning@92 {{use of the 'nodiscard' attribute is a C++2a > extension}} > +// expected-warning@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@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits