On 3/4/21 4:53 AM, Jakub Jelinek wrote:
Hi!

In the P1771R1 changes JeanHeyd reverted part of Alex' PR88146 fix,
but that seems to be incorrect to me.
Where P1771R1 suggests warnings for [[nodiscard]] on constructors is
handled in a different place - in particular the TARGET_EXPR handling
of convert_to_void.  When we have CALL_EXPR of a ctor, on most arches
that call has void return type and so returns early, and on arm where
the ctor returns the this pointer it is undesirable to warn as it warns
about all ctor calls, not just the ones where it should warn.

The P1771R1 changes added a test for this, but as it was given *.c
extension rather than *.C, the test was never run and so this didn't get
spotted immediately.  The test also had a bug, (?n) can't be used
in dg-warning/dg-error because those are implemented by prepending
some regexp before the user provided one and (?n) must come at the start
of the regexp.  Furthermore, while -ftrack-macro-expansion=0 is useful
in one nodiscard test which uses macros, I don't see how it would be
relevant to all the other cpp2a/nodiscard* tests which don't use any
macros.

Bootstrapped/regtested on x86_64-linux, i686-linux, armv7hl-linux and
aarch64-linux, ok for trunk?

OK.

2021-03-03  Jakub Jelinek  <ja...@redhat.com>

        PR c++/88146
        PR c++/99362
gcc/cp/
        * cvt.c (convert_to_void): Revert 2019-10-17 changes.  Clarify
        comment.
gcc/testsuite/
        * g++.dg/cpp2a/nodiscard-constructor.c: Renamed to ...
        * g++.dg/cpp2a/nodiscard-constructor1.C: ... this.  Remove
        -ftrack-macro-expansion=0 from dg-options.  Don't use (?n) in
        dg-warning regexps, instead replace .* with \[^\n\r]*.
        * g++.dg/cpp2a/nodiscard-constructor2.C: New test.
        * g++.dg/cpp2a/nodiscard-reason-only-one.C: Remove
        -ftrack-macro-expansion=0 from dg-options.
        * g++.dg/cpp2a/nodiscard-reason-nonstring.C: Likewise.
        * g++.dg/cpp2a/nodiscard-once.C: Likewise.

--- gcc/cp/cvt.c.jj     2021-01-26 08:57:46.469172105 +0100
+++ gcc/cp/cvt.c        2021-03-03 20:22:35.849086890 +0100
@@ -1231,12 +1231,14 @@ convert_to_void (tree expr, impl_conv_vo
      case CALL_EXPR:   /* We have a special meaning for volatile void fn().  */
        /* cdtors may return this or void, depending on
         targetm.cxx.cdtor_returns_this, but this shouldn't affect our
-        decisions here: neither nodiscard warnings (nodiscard cdtors
-        are nonsensical), nor should any constexpr or template
-        instantiations be affected by an ABI property that is, or at
-        least ought to be transparent to the language.  */
+        decisions here: neither nodiscard warnings (nodiscard dtors
+        are nonsensical and ctors have a different behavior with that
+        attribute that is handled in the TARGET_EXPR case), nor should
+        any constexpr or template instantiations be affected by an ABI
+        property that is, or at least ought to be transparent to the
+        language.  */
        if (tree fn = cp_get_callee_fndecl_nofold (expr))
-       if (DECL_DESTRUCTOR_P (fn))
+       if (DECL_CONSTRUCTOR_P (fn) || DECL_DESTRUCTOR_P (fn))
          return expr;
if (complain & tf_warning)
--- gcc/testsuite/g++.dg/cpp2a/nodiscard-constructor.c.jj       2020-07-28 
15:39:10.008756228 +0200
+++ gcc/testsuite/g++.dg/cpp2a/nodiscard-constructor.c  2021-03-03 
20:28:46.095046953 +0100
@@ -1,13 +0,0 @@
-/* nodiscard attribute tests  */
-/* { dg-do compile { target c++20 } } */
-/* { dg-options "-O -ftrack-macro-expansion=0" } */
-
-struct A { [[nodiscard("bad constructor")]] A() {} };
-struct B { [[nodiscard]] B() {} };
-
-void
-test (void)
-{
-  A{}; /* { dg-warning "(?n)nodiscard.*bad constructor" } */
-  B{}; /* { dg-warning "(?n)nodiscard" } */
-}
--- gcc/testsuite/g++.dg/cpp2a/nodiscard-constructor1.C.jj      2021-03-03 
20:28:39.364120391 +0100
+++ gcc/testsuite/g++.dg/cpp2a/nodiscard-constructor1.C 2021-03-03 
20:37:53.186077373 +0100
@@ -0,0 +1,13 @@
+/* nodiscard attribute tests  */
+/* { dg-do compile { target c++20 } } */
+/* { dg-options "-O" } */
+
+struct A { [[nodiscard("bad constructor")]] A() {} };
+struct B { [[nodiscard]] B() {} };
+
+void
+test (void)
+{
+  A{}; /* { dg-warning "nodiscard\[^\n\r]*bad constructor" } */
+  B{}; /* { dg-warning "nodiscard" } */
+}
--- gcc/testsuite/g++.dg/cpp2a/nodiscard-constructor2.C.jj      2021-03-03 
20:32:02.781900801 +0100
+++ gcc/testsuite/g++.dg/cpp2a/nodiscard-constructor2.C 2021-03-03 
20:34:33.416257158 +0100
@@ -0,0 +1,17 @@
+// PR c++/99362
+// { dg-do compile { target c++20 } }
+
+struct S {
+  [[nodiscard]] S() {}
+  S(int) {}
+};
+
+int main()
+{
+  S s;
+  S();         // { dg-warning "ignoring return value" }
+  (void)(S());
+  S t = 1;
+  S(1);
+  (void)(S(1));
+}
--- gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C.jj      2020-11-10 
10:21:27.272096866 +0100
+++ gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C 2021-03-03 20:31:11.115464565 
+0100
@@ -1,6 +1,6 @@
  /* nodiscard attribute tests  */
  /* { dg-do compile { target c++20 } } */
-/* { dg-options "-O -ftrack-macro-expansion=0" } */
+/* { dg-options "-O" } */
[[nodiscard, nodiscard]] int check1 (void); // { dg-warning "specified multiple times" } --- gcc/testsuite/g++.dg/cpp2a/nodiscard-reason-only-one.C.jj 2020-07-28 15:39:10.008756228 +0200
+++ gcc/testsuite/g++.dg/cpp2a/nodiscard-reason-only-one.C      2021-03-03 
20:31:39.427155637 +0100
@@ -1,6 +1,6 @@
  /* nodiscard attribute tests  */
  /* { dg-do compile { target c++20 } } */
-/* { dg-options "-O -ftrack-macro-expansion=0" } */
+/* { dg-options "-O" } */
[[nodiscard("not", "allowed")]] int check1 (void); /* { dg-error "wrong number of arguments.\[^\n\r]*nodiscard" } */ --- gcc/testsuite/g++.dg/cpp2a/nodiscard-reason-nonstring.C.jj 2020-07-28 15:39:10.008756228 +0200
+++ gcc/testsuite/g++.dg/cpp2a/nodiscard-reason-nonstring.C     2021-03-03 
20:31:26.363298192 +0100
@@ -1,6 +1,6 @@
  /* nodiscard attribute tests  */
  /* { dg-do compile { target c++20 } } */
-/* { dg-options "-O -ftrack-macro-expansion=0" } */
+/* { dg-options "-O" } */
[[nodiscard(123)]] int check1 (void); /* { dg-error "nodiscard\[^\n\r]*must be a string constant" } */
        Jakub


Reply via email to