r367027 - Implement P1771
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 25 09:25:42 PDT 2019
On Thu, 25 Jul 2019, 08:10 Erich Keane via cfe-commits, <
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
> 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/0007ed40/attachment-0001.html>
More information about the cfe-commits
mailing list