Re: [PATCH] c++: fix noexcept checking for trivial operations [PR96090]

[Nathaniel Shead <nathanieloshead@gmail.com>]

Ping for https://gcc.gnu.org/pipermail/gcc-patches/2023-October/634626.html.

I've been made aware since constructing this patch of CWG2820, which has
a proposed resolution that would change the result of the testcase
'noexcept(yesthrow_t())' (and similarly for the library builtin), but as
it hasn't yet been accepted I think at least ensuring the builtin
matches the behaviour of the operator is probably still sensible.

On Sun, Oct 29, 2023 at 12:43:28PM +1100, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu.
> 
> -- >8 --
> 
> This patch stops eager folding of trivial operations (construction and
> assignment) from occurring when checking for noexceptness. This was
> previously done in PR c++/53025, but only for copy/move construction,
> and the __is_nothrow_xible builtins did not receive the same treatment
> when they were added.
> 
> To handle `is_nothrow_default_constructible`, the patch also ensures
> that when no parameters are passed we do value initialisation instead of
> just building the constructor call: in particular, value-initialisation
> doesn't necessarily actually invoke the constructor for trivial default
> constructors, and so we need to handle this case as well.
> 
> 	PR c++/96090
> 	PR c++/100470
> 
> gcc/cp/ChangeLog:
> 
> 	* call.cc (build_over_call): Prevent folding of trivial special
> 	members when checking for noexcept.
> 	* method.cc (constructible_expr): Perform value-initialisation
> 	for empty parameter lists.
> 	(is_nothrow_xible): Treat as noexcept operator.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/noexcept81.C: New test.
> 	* g++.dg/ext/is_nothrow_constructible7.C: New test.
> 	* g++.dg/ext/is_nothrow_constructible8.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>  gcc/cp/call.cc                                | 17 ++---
>  gcc/cp/method.cc                              | 19 ++++--
>  gcc/testsuite/g++.dg/cpp0x/noexcept81.C       | 36 +++++++++++
>  .../g++.dg/ext/is_nothrow_constructible7.C    | 20 ++++++
>  .../g++.dg/ext/is_nothrow_constructible8.C    | 63 +++++++++++++++++++
>  5 files changed, 141 insertions(+), 14 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/noexcept81.C
>  create mode 100644 gcc/testsuite/g++.dg/ext/is_nothrow_constructible7.C
>  create mode 100644 gcc/testsuite/g++.dg/ext/is_nothrow_constructible8.C
> 
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index c1fb8807d3f..ac02b0633ed 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -10231,15 +10231,16 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
>    /* Avoid actually calling copy constructors and copy assignment operators,
>       if possible.  */
>  
> -  if (! flag_elide_constructors && !force_elide)
> +  if (!force_elide 
> +      && (!flag_elide_constructors
> +	  /* It's unsafe to elide the operation when handling
> +	     a noexcept-expression, it may evaluate to the wrong
> +	     value (c++/53025, c++/96090).  */
> +	  || cp_noexcept_operand != 0))
>      /* Do things the hard way.  */;
> -  else if (cand->num_convs == 1 
> -           && (DECL_COPY_CONSTRUCTOR_P (fn) 
> -               || DECL_MOVE_CONSTRUCTOR_P (fn))
> -	   /* It's unsafe to elide the constructor when handling
> -	      a noexcept-expression, it may evaluate to the wrong
> -	      value (c++/53025).  */
> -	   && (force_elide || cp_noexcept_operand == 0))
> +  else if (cand->num_convs == 1
> +	   && (DECL_COPY_CONSTRUCTOR_P (fn)
> +	       || DECL_MOVE_CONSTRUCTOR_P (fn)))
>      {
>        tree targ;
>        tree arg = argarray[num_artificial_parms_for (fn)];
> diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
> index a70dd5d6adc..3c978e2369d 100644
> --- a/gcc/cp/method.cc
> +++ b/gcc/cp/method.cc
> @@ -2091,6 +2091,7 @@ constructible_expr (tree to, tree from)
>  {
>    tree expr;
>    cp_unevaluated cp_uneval_guard;
> +  const int len = TREE_VEC_LENGTH (from);
>    if (CLASS_TYPE_P (to))
>      {
>        tree ctype = to;
> @@ -2098,11 +2099,16 @@ constructible_expr (tree to, tree from)
>        if (!TYPE_REF_P (to))
>  	to = cp_build_reference_type (to, /*rval*/false);
>        tree ob = build_stub_object (to);
> -      vec_alloc (args, TREE_VEC_LENGTH (from));
> -      for (tree arg : tree_vec_range (from))
> -	args->quick_push (build_stub_object (arg));
> -      expr = build_special_member_call (ob, complete_ctor_identifier, &args,
> -					ctype, LOOKUP_NORMAL, tf_none);
> +      if (len == 0)
> +	expr = build_value_init (ctype, tf_none);
> +      else
> +	{
> +	  vec_alloc (args, TREE_VEC_LENGTH (from));
> +	  for (tree arg : tree_vec_range (from))
> +	    args->quick_push (build_stub_object (arg));
> +	  expr = build_special_member_call (ob, complete_ctor_identifier, &args,
> +					    ctype, LOOKUP_NORMAL, tf_none);
> +	}
>        if (expr == error_mark_node)
>  	return error_mark_node;
>        /* The current state of the standard vis-a-vis LWG 2116 is that
> @@ -2120,7 +2126,6 @@ constructible_expr (tree to, tree from)
>      }
>    else
>      {
> -      const int len = TREE_VEC_LENGTH (from);
>        if (len == 0)
>  	return build_value_init (strip_array_types (to), tf_none);
>        if (len > 1)
> @@ -2216,7 +2221,9 @@ is_trivially_xible (enum tree_code code, tree to, tree from)
>  bool
>  is_nothrow_xible (enum tree_code code, tree to, tree from)
>  {
> +  ++cp_noexcept_operand;
>    tree expr = is_xible_helper (code, to, from, /*trivial*/false);
> +  --cp_noexcept_operand;
>    if (expr == NULL_TREE || expr == error_mark_node)
>      return false;
>    return expr_noexcept_p (expr, tf_none);
> diff --git a/gcc/testsuite/g++.dg/cpp0x/noexcept81.C b/gcc/testsuite/g++.dg/cpp0x/noexcept81.C
> new file mode 100644
> index 00000000000..a1481613b5d
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/noexcept81.C
> @@ -0,0 +1,36 @@
> +// { dg-do compile { target c++11 } }
> +// PR c++/96090
> +
> +struct yesthrow_t {
> +  yesthrow_t()                              noexcept(false) = default;
> +  yesthrow_t(const yesthrow_t&)             noexcept(false) = default;
> +  yesthrow_t(yesthrow_t&&)                  noexcept(false) = default;
> +  yesthrow_t& operator=(const yesthrow_t&)  noexcept(false) = default;
> +  yesthrow_t& operator=(yesthrow_t&&)       noexcept(false) = default;
> +};
> +
> +yesthrow_t yes;
> +static_assert(not noexcept(yesthrow_t(static_cast<const yesthrow_t&>(yes))), "");
> +static_assert(not noexcept(yesthrow_t(static_cast<yesthrow_t&&>(yes))), "");
> +static_assert(not noexcept(yes = static_cast<const yesthrow_t&>(yes)), "");
> +static_assert(not noexcept(yes = static_cast<yesthrow_t&&>(yes)), "");
> +
> +// Note: this is value-initialisation, and thus by [dcl.init.general] p9
> +// a trivial non-user-provided non-deleted default constructor is not called.
> +static_assert(noexcept(yesthrow_t()), "");
> +
> +struct nothrow_t {
> +  nothrow_t()                             noexcept(true) = default;
> +  nothrow_t(const nothrow_t&)             noexcept(true) = default;
> +  nothrow_t(nothrow_t&&)                  noexcept(true) = default;
> +  nothrow_t& operator=(const nothrow_t&)  noexcept(true) = default;
> +  nothrow_t& operator=(nothrow_t&&)       noexcept(true) = default;
> +};
> +
> +nothrow_t no;
> +static_assert(noexcept(nothrow_t()), "");
> +static_assert(noexcept(nothrow_t(static_cast<const nothrow_t&>(no))), "");
> +static_assert(noexcept(nothrow_t(static_cast<nothrow_t&&>(no))), "");
> +static_assert(noexcept(no = static_cast<const nothrow_t&>(no)), "");
> +static_assert(noexcept(no = static_cast<nothrow_t&&>(no)), "");
> +
> diff --git a/gcc/testsuite/g++.dg/ext/is_nothrow_constructible7.C b/gcc/testsuite/g++.dg/ext/is_nothrow_constructible7.C
> new file mode 100644
> index 00000000000..b63b13ac52f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ext/is_nothrow_constructible7.C
> @@ -0,0 +1,20 @@
> +// { dg-do compile { target c++11 } }
> +// PR c++/100470
> +
> +struct S1{
> +    S1(S1&&) noexcept(false);
> +};
> +struct S2{
> +    S2(S2&&) noexcept(false) = default;
> +};
> +struct S3{
> +    S3(S3&&) noexcept(false){}
> +};
> +struct S4{
> +    S4(S4&&) = default;
> +};
> +
> +static_assert(!__is_nothrow_constructible(S1, S1), "");
> +static_assert(!__is_nothrow_constructible(S2, S2), "");
> +static_assert(!__is_nothrow_constructible(S3, S3), "");
> +static_assert( __is_nothrow_constructible(S4, S4), "");
> diff --git a/gcc/testsuite/g++.dg/ext/is_nothrow_constructible8.C b/gcc/testsuite/g++.dg/ext/is_nothrow_constructible8.C
> new file mode 100644
> index 00000000000..f23d48fa888
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ext/is_nothrow_constructible8.C
> @@ -0,0 +1,63 @@
> +// { dg-do compile { target c++11 } }
> +// PR c++/96090
> +
> +template <typename T>
> +constexpr bool is_nothrow_default_constructible_v
> +  = __is_nothrow_constructible(T);
> +template <typename T>
> +constexpr bool is_nothrow_copy_constructible_v
> +  = __is_nothrow_constructible(T, const T&);
> +template <typename T>
> +constexpr bool is_nothrow_move_constructible_v
> +  = __is_nothrow_constructible(T, T&&);
> +template <typename T>
> +constexpr bool is_nothrow_copy_assignable_v
> +  = __is_nothrow_assignable(T, const T&);
> +template <typename T>
> +constexpr bool is_nothrow_move_assignable_v
> +  = __is_nothrow_assignable(T, T&&);
> +
> +struct yesthrow_t {
> +  yesthrow_t()                              noexcept(false) = default;
> +  yesthrow_t(const yesthrow_t&)             noexcept(false) = default;
> +  yesthrow_t(yesthrow_t&&)                  noexcept(false) = default;
> +  yesthrow_t& operator=(const yesthrow_t&)  noexcept(false) = default;
> +  yesthrow_t& operator=(yesthrow_t&&)       noexcept(false) = default;
> +};
> +
> +static_assert(not is_nothrow_copy_constructible_v<yesthrow_t>, "");
> +static_assert(not is_nothrow_copy_assignable_v<yesthrow_t>, "");
> +static_assert(not is_nothrow_move_constructible_v<yesthrow_t>, "");
> +static_assert(not is_nothrow_move_assignable_v<yesthrow_t>, "");
> +
> +// Note: by [meta.unary.prop] p9 this should be value-initialisation,
> +// and thus by [dcl.init.general] p9 a trivial non-user-provided
> +// non-deleted default constructor is not called.
> +static_assert(is_nothrow_default_constructible_v<yesthrow_t>, "");
> +
> +struct nothrow_t {
> +  nothrow_t()                             noexcept(true) = default;
> +  nothrow_t(const nothrow_t&)             noexcept(true) = default;
> +  nothrow_t(nothrow_t&&)                  noexcept(true) = default;
> +  nothrow_t& operator=(const nothrow_t&)  noexcept(true) = default;
> +  nothrow_t& operator=(nothrow_t&&)       noexcept(true) = default;
> +};
> +
> +static_assert(is_nothrow_default_constructible_v<nothrow_t>, "");
> +static_assert(is_nothrow_copy_constructible_v<nothrow_t>, "");
> +static_assert(is_nothrow_copy_assignable_v<nothrow_t>, "");
> +static_assert(is_nothrow_move_constructible_v<nothrow_t>, "");
> +static_assert(is_nothrow_move_assignable_v<nothrow_t>, "");
> +
> +struct A { A() noexcept(false) = default; };
> +struct B { B(const B&) noexcept(false) = default; };
> +struct C { C(C&&) noexcept(false) = default; };
> +struct D { D& operator=(const D&) noexcept(false) = default; };
> +struct E { E& operator=(E&&) noexcept(false) = default; };
> +
> +static_assert(is_nothrow_default_constructible_v<A>, "");  // see above
> +static_assert(not is_nothrow_copy_constructible_v<B>, "");
> +static_assert(not is_nothrow_move_constructible_v<C>, "");
> +static_assert(not is_nothrow_copy_assignable_v<D>, "");
> +static_assert(not is_nothrow_move_assignable_v<E>, "");
> +
> -- 
> 2.42.0
>