r366626 - Implement P1301R4, which allows specifying an optional message on the [[nodiscard]] attribute.

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Sun Jul 21 04:40:09 PDT 2019


On Sat, 20 Jul 2019 at 09:55, Aaron Ballman via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: aaronballman
> Date: Sat Jul 20 00:56:34 2019
> New Revision: 366626
>
> URL: http://llvm.org/viewvc/llvm-project?rev=366626&view=rev
> Log:
> Implement P1301R4, which allows specifying an optional message on the
> [[nodiscard]] attribute.
>
> This also bumps the attribute feature test value and introduces the notion
> of a C++2a extension warning.
>
> 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/Sema/SemaDeclAttr.cpp
>     cfe/trunk/lib/Sema/SemaStmt.cpp
>     cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp
>     cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
>     cfe/trunk/test/Preprocessor/has_attribute.cpp
>     cfe/trunk/test/Sema/c2x-nodiscard.c
>     cfe/trunk/test/SemaCXX/cxx11-attr-print.cpp
>
> Modified: cfe/trunk/include/clang/Basic/Attr.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=366626&r1=366625&r2=366626&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Sat Jul 20 00:56:34 2019
> @@ -2335,10 +2335,11 @@ def WarnUnused : InheritableAttr {
>  }
>
>  def WarnUnusedResult : InheritableAttr {
> -  let Spellings = [CXX11<"", "nodiscard", 201603>, C2x<"", "nodiscard">,
> +  let Spellings = [CXX11<"", "nodiscard", 201907>, C2x<"", "nodiscard">,
>

We shouldn't bump the version until we also implement P1771R1, as this
number indicates that both papers are implemented.


>                     CXX11<"clang", "warn_unused_result">,
>                     GCC<"warn_unused_result">];
>    let Subjects = SubjectList<[ObjCMethod, Enum, Record, FunctionLike]>;
> +  let Args = [StringArgument<"Message", 1>];
>    let Documentation = [WarnUnusedResultsDocs];
>  }
>
>
> Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=366626&r1=366625&r2=366626&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
> +++ cfe/trunk/include/clang/Basic/AttrDocs.td Sat Jul 20 00:56:34 2019
> @@ -1482,6 +1482,13 @@ generated when a function or its return
>  potentially-evaluated discarded-value expression that is not explicitly
> cast to
>  `void`.
>
> +A string literal may optionally be provided to the attribute, which will
> be
> +reproduced in any resulting diagnostics. Redeclarations using different
> forms
> +of the attribute (with or without the string literal or with different
> string
> +literal contents) are allowed. If there are redeclarations of the entity
> with
> +differing string literals, it is unspecified which one will be used by
> Clang
> +in any resulting diagnostics.
> +
>  .. code-block: c++
>    struct [[nodiscard]] error_info { /*...*/ };
>    error_info enable_missile_safety_mode();
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=366626&r1=366625&r2=366626&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sat Jul 20
> 00:56:34 2019
> @@ -7436,6 +7436,9 @@ def warn_side_effects_typeid : Warning<
>  def warn_unused_result : Warning<
>    "ignoring return value of function declared with %0 attribute">,
>    InGroup<UnusedResult>;
> +def warn_unused_result_msg : Warning<
> +  "ignoring return value of function declared with %0 attribute: %1">,
> +  InGroup<UnusedResult>;
>  def warn_unused_volatile : Warning<
>    "expression result unused; assign into a variable to force a volatile
> load">,
>    InGroup<DiagGroup<"unused-volatile-lvalue">>;
> @@ -7444,6 +7447,8 @@ def ext_cxx14_attr : Extension<
>    "use of the %0 attribute is a C++14 extension">, InGroup<CXX14>;
>  def ext_cxx17_attr : Extension<
>    "use of the %0 attribute is a C++17 extension">, InGroup<CXX17>;
> +def ext_cxx2a_attr : Extension<
> +  "use of the %0 attribute is a C++2a extension">, InGroup<CXX2a>;
>
>  def warn_unused_comparison : Warning<
>    "%select{equality|inequality|relational|three-way}0 comparison result
> unused">,
>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=366626&r1=366625&r2=366626&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Sat Jul 20 00:56:34 2019
> @@ -2841,14 +2841,30 @@ static void handleWarnUnusedResult(Sema
>        return;
>      }
>
> -  // If this is spelled as the standard C++17 attribute, but not in
> C++17, warn
> -  // about using it as an extension.
> -  if (!S.getLangOpts().CPlusPlus17 && AL.isCXX11Attribute() &&
> -      !AL.getScopeName())
> -    S.Diag(AL.getLoc(), diag::ext_cxx17_attr) << AL;
> +  StringRef Str;
> +  if ((AL.isCXX11Attribute() || AL.isC2xAttribute()) &&
> !AL.getScopeName()) {
> +    // If this is spelled as the standard C++17 attribute, but not in
> C++17,
> +    // warn about using it as an extension. If there are attribute
> arguments,
> +    // then claim it's a C++2a extension instead.
> +    // FIXME: If WG14 does not seem likely to adopt the same feature, add
> an
> +    // extension warning for C2x mode.
> +    const LangOptions &LO = S.getLangOpts();
> +    if (AL.getNumArgs() == 1) {
> +      if (LO.CPlusPlus && !LO.CPlusPlus2a)
> +        S.Diag(AL.getLoc(), diag::ext_cxx2a_attr) << AL;
> +
> +      // Since this this is spelled [[nodiscard]], get the optional string
> +      // literal. If in C++ mode, but not in C++2a mode, diagnose as an
> +      // extension.
> +      // FIXME: C2x should support this feature as well, even as an
> extension.
> +      if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, nullptr))
> +        return;
> +    } else if (LO.CPlusPlus && !LO.CPlusPlus17)
> +      S.Diag(AL.getLoc(), diag::ext_cxx17_attr) << AL;
> +  }
>
>    D->addAttr(::new (S.Context)
> -             WarnUnusedResultAttr(AL.getRange(), S.Context,
> +             WarnUnusedResultAttr(AL.getRange(), S.Context, Str,
>                                    AL.getAttributeSpellingListIndex()));
>  }
>
>
> Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=366626&r1=366625&r2=366626&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaStmt.cpp Sat Jul 20 00:56:34 2019
> @@ -258,8 +258,13 @@ void Sema::DiagnoseUnusedExprResult(cons
>      if (E->getType()->isVoidType())
>        return;
>
> -    if (const Attr *A = CE->getUnusedResultAttr(Context)) {
> -      Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
> +    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;
>        return;
>      }
>
> @@ -290,7 +295,11 @@ void Sema::DiagnoseUnusedExprResult(cons
>      const ObjCMethodDecl *MD = ME->getMethodDecl();
>      if (MD) {
>        if (const auto *A = MD->getAttr<WarnUnusedResultAttr>()) {
> -        Diag(Loc, diag::warn_unused_result) << A << R1 << R2;
> +        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;
>          return;
>        }
>      }
>
> Modified: cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp?rev=366626&r1=366625&r2=366626&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp
> (original)
> +++ cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp Sat Jul
> 20 00:56:34 2019
> @@ -1,8 +1,8 @@
> -// RUN: %clang_cc1 -fsyntax-only -std=c++1z -verify %s
> +// RUN: %clang_cc1 -fsyntax-only -std=c++2a -verify %s
>
>  struct [[nodiscard]] S1 {}; // ok
>  struct [[nodiscard nodiscard]] S2 {}; // expected-error {{attribute
> 'nodiscard' cannot appear multiple times in an attribute specifier}}
> -struct [[nodiscard("Wrong")]] S3 {}; // expected-error {{'nodiscard'
> cannot have an argument list}}
> +struct [[nodiscard("Wrong")]] S3 {};
>
>  [[nodiscard]] int f();
>  enum [[nodiscard]] E {};
>
> 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=366626&r1=366625&r2=366626&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 Sat Jul
> 20 00:56:34 2019
> @@ -1,5 +1,6 @@
> +// RUN: %clang_cc1 -fsyntax-only -std=c++2a -verify -Wc++2a-extensions %s
>  // RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify -Wc++17-extensions %s
> -// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify -DEXT
> -Wc++17-extensions %s
> +// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify -DEXT
> -Wc++17-extensions -Wc++2a-extensions %s
>
>  struct [[nodiscard]] S {};
>  S get_s();
> @@ -61,10 +62,33 @@ void f() {
>  }
>  } // namespace PR31526
>
> +struct [[nodiscard("reason")]] ReasonStruct {};
> +struct LaterReason;
> +struct [[nodiscard("later reason")]] LaterReason {};
> +
> +ReasonStruct get_reason();
> +LaterReason get_later_reason();
> +[[nodiscard("another reason")]] int another_reason();
> +
> +[[nodiscard("conflicting reason")]] int conflicting_reason();
> +[[nodiscard("special reason")]] int conflicting_reason();
> +
> +void cxx2a_use() {
> +  get_reason(); // expected-warning {{ignoring return value of function
> declared with 'nodiscard' attribute: reason}}
> +  get_later_reason(); // expected-warning {{ignoring return value of
> function declared with 'nodiscard' attribute: later reason}}
> +  another_reason(); // expected-warning {{ignoring return value of
> function declared with 'nodiscard' attribute: another reason}}
> +  conflicting_reason(); // expected-warning {{ignoring return value of
> function declared with 'nodiscard' attribute: special reason}}
> +}
> +
>  #ifdef EXT
> -// expected-warning at 4 {{use of the 'nodiscard' attribute is a C++17
> extension}}
> -// expected-warning at 8 {{use of the 'nodiscard' attribute is a C++17
> extension}}
> -// expected-warning at 11 {{use of the 'nodiscard' attribute is a C++17
> extension}}
> +// 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}}
>  // expected-warning at 12 {{use of the 'nodiscard' attribute is a C++17
> extension}}
> -// expected-warning at 28 {{use of the 'nodiscard' attribute is a C++17
> extension}}
> +// expected-warning at 13 {{use of the 'nodiscard' attribute is a C++17
> extension}}
> +// expected-warning at 29 {{use of the 'nodiscard' attribute is a C++17
> extension}}
> +// expected-warning at 65 {{use of the 'nodiscard' attribute is a C++2a
> extension}}
> +// expected-warning at 67 {{use of the 'nodiscard' attribute is a C++2a
> extension}}
> +// 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}}
>  #endif
>
> Modified: cfe/trunk/test/Preprocessor/has_attribute.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/has_attribute.cpp?rev=366626&r1=366625&r2=366626&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Preprocessor/has_attribute.cpp (original)
> +++ cfe/trunk/test/Preprocessor/has_attribute.cpp Sat Jul 20 00:56:34 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/test/Sema/c2x-nodiscard.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/c2x-nodiscard.c?rev=366626&r1=366625&r2=366626&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Sema/c2x-nodiscard.c (original)
> +++ cfe/trunk/test/Sema/c2x-nodiscard.c Sat Jul 20 00:56:34 2019
> @@ -6,10 +6,12 @@ struct [[nodiscard]] S1 { // ok
>  struct [[nodiscard nodiscard]] S2 { // expected-error {{attribute
> 'nodiscard' cannot appear multiple times in an attribute specifier}}
>    int i;
>  };
> -struct [[nodiscard("Wrong")]] S3 { // expected-error {{'nodiscard' cannot
> have an argument list}}
> +struct [[nodiscard("Wrong")]] S3 { // FIXME: may need an extension
> warning.
>    int i;
>  };
>
> +struct S3 get_s3(void);
> +
>  [[nodiscard]] int f1(void);
>  enum [[nodiscard]] E1 { One };
>
> @@ -27,11 +29,13 @@ enum E2 get_e(void);
>
>  void f2(void) {
>    get_s(); // expected-warning {{ignoring return value of function
> declared with 'nodiscard' attribute}}
> +  get_s3(); // expected-warning {{ignoring return value of function
> declared with 'nodiscard' attribute: Wrong}}
>    get_i(); // expected-warning {{ignoring return value of function
> declared with 'nodiscard' attribute}}
>    get_e(); // expected-warning {{ignoring return value of function
> declared with 'nodiscard' attribute}}
>
>    // Okay, warnings are not encouraged
>    (void)get_s();
> +  (void)get_s3();
>    (void)get_i();
>    (void)get_e();
>  }
>
> Modified: cfe/trunk/test/SemaCXX/cxx11-attr-print.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx11-attr-print.cpp?rev=366626&r1=366625&r2=366626&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/cxx11-attr-print.cpp (original)
> +++ cfe/trunk/test/SemaCXX/cxx11-attr-print.cpp Sat Jul 20 00:56:34 2019
> @@ -37,13 +37,15 @@ void foo() __attribute__((const));
>  // CHECK: void bar() __attribute__((__const));
>  void bar() __attribute__((__const));
>
> -// CHECK: int f1() __attribute__((warn_unused_result));
> +// FIXME: It's unfortunate that the string literal prints with the below
> three
> +// cases given that the string is only exposed via the [[nodiscard]]
> spelling.
> +// CHECK: int f1() __attribute__((warn_unused_result("")));
>  int f1() __attribute__((warn_unused_result));
>
> -// CHECK: {{\[}}[clang::warn_unused_result]];
> +// CHECK: {{\[}}[clang::warn_unused_result("")]];
>  int f2 [[clang::warn_unused_result]] ();
>
> -// CHECK: {{\[}}[gnu::warn_unused_result]];
> +// CHECK: {{\[}}[gnu::warn_unused_result("")]];
>  int f3 [[gnu::warn_unused_result]] ();
>
>  // FIXME: ast-print need to print C++11
>
>
> _______________________________________________
> 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/20190721/7dc31c3b/attachment-0001.html>


More information about the cfe-commits mailing list