diff mbox series

[PR85569] skip constexpr target_expr constructor dummy type conversion

Message ID or1s7csmv8.fsf@lxoliva.fsfla.org
State New
Headers show
Series [PR85569] skip constexpr target_expr constructor dummy type conversion | expand

Commit Message

Alexandre Oliva Nov. 22, 2018, 11:39 p.m. UTC
The testcase is the work-around testcase for the PR; even that had
started failing.  The problem was that, when unqualifying the type of
a TARGET_EXPR, we'd create a variant of the type, then request the
conversion of the TARGET_EXPR_INITIAL to that variant type.  Though
the types are different pointer-wise, they're the same_type_p, so the
resulting modified expr compares cp_tree_equal to the original, which
maybe_constant_value flags as an error.  There's no reason to
construct an alternate TARGET_EXPR or CONSTRUCTOR just because of an
equivalent type, except for another spot that expected pointer
equality that would no longer be satisfied.  Without relaxing the
assert in constexpr_call_hasher::equal, g++.robertl/eb73.C would
trigger an assertion failure.

Regstrapped on i686- and x86_64-linux-gnu.  Ok to install?


for  gcc/cp/ChangeLog

	PR c++/85569
	* constexpr.c (adjust_temp_type): Test for type equality with
	same_type_p.

for  gcc/testsuite

	PR c++/85569
	* g++.dg/cpp1z/pr85569.C: New.
---
 gcc/cp/constexpr.c                   |    4 +
 gcc/testsuite/g++.dg/cpp1z/pr85569.C |   93 ++++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/pr85569.C

Comments

Jason Merrill Nov. 27, 2018, 11:41 p.m. UTC | #1
On 11/22/18 6:39 PM, Alexandre Oliva wrote:
> The testcase is the work-around testcase for the PR; even that had
> started failing.  The problem was that, when unqualifying the type of
> a TARGET_EXPR, we'd create a variant of the type, then request the
> conversion of the TARGET_EXPR_INITIAL to that variant type.  Though
> the types are different pointer-wise, they're the same_type_p, so the
> resulting modified expr compares cp_tree_equal to the original, which
> maybe_constant_value flags as an error.  There's no reason to
> construct an alternate TARGET_EXPR or CONSTRUCTOR just because of an
> equivalent type, except for another spot that expected pointer
> equality that would no longer be satisfied.  Without relaxing the
> assert in constexpr_call_hasher::equal, g++.robertl/eb73.C would
> trigger an assertion failure.
> 
> Regstrapped on i686- and x86_64-linux-gnu.  Ok to install?
> 
> 
> for  gcc/cp/ChangeLog
> 
> 	PR c++/85569
> 	* constexpr.c (adjust_temp_type): Test for type equality with
> 	same_type_p.
> 
> for  gcc/testsuite
> 
> 	PR c++/85569
> 	* g++.dg/cpp1z/pr85569.C: New.
> ---
>   gcc/cp/constexpr.c                   |    4 +
>   gcc/testsuite/g++.dg/cpp1z/pr85569.C |   93 ++++++++++++++++++++++++++++++++++
>   2 files changed, 95 insertions(+), 2 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp1z/pr85569.C
> 
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index 92fd2b2d9d59..bb5d1301b332 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -1060,7 +1060,7 @@ constexpr_call_hasher::equal (constexpr_call *lhs, constexpr_call *rhs)
>       {
>         tree lhs_arg = TREE_VALUE (lhs_bindings);
>         tree rhs_arg = TREE_VALUE (rhs_bindings);
> -      gcc_assert (TREE_TYPE (lhs_arg) == TREE_TYPE (rhs_arg));
> +      gcc_assert (same_type_p (TREE_TYPE (lhs_arg), TREE_TYPE (rhs_arg)));
>         if (!cp_tree_equal (lhs_arg, rhs_arg))
>           return false;
>         lhs_bindings = TREE_CHAIN (lhs_bindings);
> @@ -1276,7 +1276,7 @@ cxx_eval_builtin_function_call (const constexpr_ctx *ctx, tree t, tree fun,
>   static tree
>   adjust_temp_type (tree type, tree temp)
>   {
> -  if (TREE_TYPE (temp) == type)
> +  if (TREE_TYPE (temp) == type || same_type_p (TREE_TYPE (temp), type))
>       return temp;
>     /* Avoid wrapping an aggregate value in a NOP_EXPR.  */

Hmm, I'm a bit uneasy about this change, but it does make sense to 
follow cp_tree_equal.

Let's replace the == comparison rather than supplement it.  OK with that 
change.

Jason
Alexandre Oliva Dec. 5, 2018, 6:34 a.m. UTC | #2
On Nov 27, 2018, Jason Merrill <jason@redhat.com> wrote:

> Let's replace the == comparison rather than supplement it.  OK with
> that change.

Thanks, here's what I (re)tested and will install eventually.


[PR85569] skip constexpr target_expr constructor dummy type conversion

From: Alexandre Oliva <aoliva@redhat.com>

The testcase is the work-around testcase for the PR; even that had
started failing.  The problem was that, when unqualifying the type of
a TARGET_EXPR, we'd create a variant of the type, then request the
conversion of the TARGET_EXPR_INITIAL to that variant type.  Though
the types are different pointer-wise, they're the same_type_p, so the
resulting modified expr compares cp_tree_equal to the original, which
maybe_constant_value flags as an error.  There's no reason to
construct an alternate TARGET_EXPR or CONSTRUCTOR just because of an
equivalent type, except for another spot that expected pointer
equality that would no longer be satisfied.  Without relaxing the
assert in constexpr_call_hasher::equal, g++.robertl/eb73.C would
trigger an assertion failure.


for  gcc/cp/ChangeLog

	PR c++/85569
	* constexpr.c (adjust_temp_type): Test for type equality with
	same_type_p.
	(constexpr_call_hasher::equal): Likewise.

for  gcc/testsuite

	PR c++/85569
	* g++.dg/cpp1z/pr85569.C: New.
---
 gcc/cp/constexpr.c                   |    4 +
 gcc/testsuite/g++.dg/cpp1z/pr85569.C |   93 ++++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/pr85569.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 92fd2b2d9d59..a668d14e8bf5 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1060,7 +1060,7 @@ constexpr_call_hasher::equal (constexpr_call *lhs, constexpr_call *rhs)
     {
       tree lhs_arg = TREE_VALUE (lhs_bindings);
       tree rhs_arg = TREE_VALUE (rhs_bindings);
-      gcc_assert (TREE_TYPE (lhs_arg) == TREE_TYPE (rhs_arg));
+      gcc_assert (same_type_p (TREE_TYPE (lhs_arg), TREE_TYPE (rhs_arg)));
       if (!cp_tree_equal (lhs_arg, rhs_arg))
         return false;
       lhs_bindings = TREE_CHAIN (lhs_bindings);
@@ -1276,7 +1276,7 @@ cxx_eval_builtin_function_call (const constexpr_ctx *ctx, tree t, tree fun,
 static tree
 adjust_temp_type (tree type, tree temp)
 {
-  if (TREE_TYPE (temp) == type)
+  if (same_type_p (TREE_TYPE (temp), type))
     return temp;
   /* Avoid wrapping an aggregate value in a NOP_EXPR.  */
   if (TREE_CODE (temp) == CONSTRUCTOR)
diff --git a/gcc/testsuite/g++.dg/cpp1z/pr85569.C b/gcc/testsuite/g++.dg/cpp1z/pr85569.C
new file mode 100644
index 000000000000..aec543041a0f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/pr85569.C
@@ -0,0 +1,93 @@
+// { dg-do compile { target c++17 } }
+
+#include <utility>
+#include <tuple>
+
+#define LIFT_FWD(x) std::forward<decltype(x)>(x)
+
+template <typename T>
+inline
+constexpr
+auto
+equal(
+  T &&t)
+{
+  return [t = std::forward<T>(t)](const auto& obj)
+    -> decltype(obj == t)
+    {
+      return obj == t;
+    };
+}
+
+template <typename F, typename T>
+struct is_tuple_invocable;
+
+template <typename F, typename ... Ts>
+struct is_tuple_invocable<F, std::tuple<Ts...>>
+{
+  using type = typename std::is_invocable<F, Ts...>::type;
+};
+
+template <typename F>
+inline
+constexpr
+auto
+compose(
+  F&& f
+)
+  noexcept
+-> F
+{
+  return std::forward<F>(f);
+}
+
+namespace detail {
+  template <typename F, typename Tail, typename ... T>
+  inline
+  constexpr
+  auto
+  compose(
+    std::true_type,
+    F&& f,
+    Tail&& tail,
+    T&& ... objs)
+  noexcept(noexcept(f(tail(std::forward<T>(objs)...))))
+  -> decltype(f(tail(std::forward<T>(objs)...)))
+  {
+    return f(tail(std::forward<T>(objs)...));
+  }
+}
+template <typename F, typename ... Fs>
+inline
+constexpr
+auto
+compose(
+  F&& f,
+  Fs&&... fs)
+{
+  return [f = std::forward<F>(f), tail = compose(std::forward<Fs>(fs)...)]
+    (auto&& ... objs)
+    -> decltype(detail::compose(typename std::is_invocable<decltype(compose(std::forward<Fs>(fs)...)), decltype(objs)...>::type{},
+                                f,
+                                compose(std::forward<Fs>(fs)...),
+                                LIFT_FWD(objs)...))
+  {
+    using tail_type = decltype(compose(std::forward<Fs>(fs)...));
+    
+#ifndef NOT_VIA_TUPLE
+    using args_type = std::tuple<decltype(objs)...>;
+    constexpr auto unitail = typename is_tuple_invocable<tail_type, args_type>::type{};
+#else
+    constexpr auto unitail = typename std::is_invocable<tail_type, decltype(objs)...>::type{};
+#endif
+
+    return detail::compose(unitail,  f, tail, LIFT_FWD(objs)...);
+  };
+}
+
+template <auto N>
+constexpr auto eq = equal(N);
+
+static_assert(compose(eq<3>,
+		      std::plus<>{})(1,2),
+              "compose is constexpr");
diff mbox series

Patch

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 92fd2b2d9d59..bb5d1301b332 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1060,7 +1060,7 @@  constexpr_call_hasher::equal (constexpr_call *lhs, constexpr_call *rhs)
     {
       tree lhs_arg = TREE_VALUE (lhs_bindings);
       tree rhs_arg = TREE_VALUE (rhs_bindings);
-      gcc_assert (TREE_TYPE (lhs_arg) == TREE_TYPE (rhs_arg));
+      gcc_assert (same_type_p (TREE_TYPE (lhs_arg), TREE_TYPE (rhs_arg)));
       if (!cp_tree_equal (lhs_arg, rhs_arg))
         return false;
       lhs_bindings = TREE_CHAIN (lhs_bindings);
@@ -1276,7 +1276,7 @@  cxx_eval_builtin_function_call (const constexpr_ctx *ctx, tree t, tree fun,
 static tree
 adjust_temp_type (tree type, tree temp)
 {
-  if (TREE_TYPE (temp) == type)
+  if (TREE_TYPE (temp) == type || same_type_p (TREE_TYPE (temp), type))
     return temp;
   /* Avoid wrapping an aggregate value in a NOP_EXPR.  */
   if (TREE_CODE (temp) == CONSTRUCTOR)
diff --git a/gcc/testsuite/g++.dg/cpp1z/pr85569.C b/gcc/testsuite/g++.dg/cpp1z/pr85569.C
new file mode 100644
index 000000000000..aec543041a0f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/pr85569.C
@@ -0,0 +1,93 @@ 
+// { dg-do compile { target c++17 } }
+
+#include <utility>
+#include <tuple>
+
+#define LIFT_FWD(x) std::forward<decltype(x)>(x)
+
+template <typename T>
+inline
+constexpr
+auto
+equal(
+  T &&t)
+{
+  return [t = std::forward<T>(t)](const auto& obj)
+    -> decltype(obj == t)
+    {
+      return obj == t;
+    };
+}
+
+template <typename F, typename T>
+struct is_tuple_invocable;
+
+template <typename F, typename ... Ts>
+struct is_tuple_invocable<F, std::tuple<Ts...>>
+{
+  using type = typename std::is_invocable<F, Ts...>::type;
+};
+
+template <typename F>
+inline
+constexpr
+auto
+compose(
+  F&& f
+)
+  noexcept
+-> F
+{
+  return std::forward<F>(f);
+}
+
+namespace detail {
+  template <typename F, typename Tail, typename ... T>
+  inline
+  constexpr
+  auto
+  compose(
+    std::true_type,
+    F&& f,
+    Tail&& tail,
+    T&& ... objs)
+  noexcept(noexcept(f(tail(std::forward<T>(objs)...))))
+  -> decltype(f(tail(std::forward<T>(objs)...)))
+  {
+    return f(tail(std::forward<T>(objs)...));
+  }
+}
+template <typename F, typename ... Fs>
+inline
+constexpr
+auto
+compose(
+  F&& f,
+  Fs&&... fs)
+{
+  return [f = std::forward<F>(f), tail = compose(std::forward<Fs>(fs)...)]
+    (auto&& ... objs)
+    -> decltype(detail::compose(typename std::is_invocable<decltype(compose(std::forward<Fs>(fs)...)), decltype(objs)...>::type{},
+                                f,
+                                compose(std::forward<Fs>(fs)...),
+                                LIFT_FWD(objs)...))
+  {
+    using tail_type = decltype(compose(std::forward<Fs>(fs)...));
+    
+#ifndef NOT_VIA_TUPLE
+    using args_type = std::tuple<decltype(objs)...>;
+    constexpr auto unitail = typename is_tuple_invocable<tail_type, args_type>::type{};
+#else
+    constexpr auto unitail = typename std::is_invocable<tail_type, decltype(objs)...>::type{};
+#endif
+
+    return detail::compose(unitail,  f, tail, LIFT_FWD(objs)...);
+  };
+}
+
+template <auto N>
+constexpr auto eq = equal(N);
+
+static_assert(compose(eq<3>,
+		      std::plus<>{})(1,2),
+              "compose is constexpr");