On 8/21/21 12:55 AM, Barrett Adair via Gcc-patches wrote:
This patch fixes AST comparison for trailing return types using dependent
sizeof/alignof/noexcept expressions as template value arguments. I believe
this bug is over a decade old, hailing from GCC 4.6. I found it over 5
years ago and sat on the repro until I had time to fix it myself.
Thanks!
The new test cases demonstrate redeclarations mistakenly parsed as
ambiguous overloads. These fail with gcc, but pass with clang. The last
test case concerns a GNU extension for alignof (hence the -Wno-pedantic).
After applying this patch, bootstrap succeeds, the new tests pass, and my
native "make -k check" outputs look essentially similar to these based on
the same commit:
https://gcc.gnu.org/pipermail/gcc-testresults/2021-August/715032.html
(except I only waited for a few thousand of the libstdc++ tests to pass).
I think it makes sense to now split the switch-case blocks here since there
isn't much shared logic left, and I have a patch to do that if desired, but
I think the patch below is the cleanest for initial review.
I studied the history and churn of the comparing_specializations hacks.
While I am still not entirely certain this is the correct change to make
there, the change seems at least superficially reasonable to me. I haven't
been able to break it yet. Perhaps someone more familiar with this area of
the compiler can weigh in. See also the existing cp_unevaluated_operand
push/pop for these EXPR codes in cp_walk_tree.
I am awaiting a response to the copyright assignment request sent to
ass...@gnu.org.
Until that goes through, you can use DCO sign-off
(https://gcc.gnu.org/dco.html).
commit fe64ca143dfb9021b4c47abb6382827c13e1f0cd
Author: Barrett Adair <barrettellisad...@gmail.com>
Date: Fri Aug 20 15:37:36 2021 -0500
Please generate your patch with git format-patch for easier application.
It also looks like the patch was corrupted by your email client; a
simple fix for that is to attach the patch instead of pasting it in the
body of your message. Or 'git help format-patch' has suggestions for
ways to make including the patch inline work.
I mostly use git send-email myself, or attachments when using send-email
would be too complicated.
Fix cp_tree_equal for template value args using
dependent sizeof/alignof/noexcept expressions
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 3c62dd74380..fce2a46cfa8 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -3903,40 +3903,41 @@ cp_tree_equal (tree t1, tree t2)
as being equivalent to anything. */
if (VAR_P (o1) && DECL_NAME (o1) == NULL_TREE
&& !DECL_RTL_SET_P (o1))
/*Nop*/;
else if (VAR_P (o2) && DECL_NAME (o2) == NULL_TREE
&& !DECL_RTL_SET_P (o2))
/*Nop*/;
else if (!cp_tree_equal (o1, o2))
return false;
return cp_tree_equal (TREE_OPERAND (t1, 1), TREE_OPERAND (t2, 1));
}
case PARM_DECL:
/* For comparing uses of parameters in late-specified return types
with an out-of-class definition of the function, but can also come
up for expressions that involve 'this' in a member function
template. */
if (comparing_specializations
+ && !cp_unevaluated_operand
This looks dangerous; I would expect it to mean that in
template <int N> struct A { };
template <class T> void f(T t1) { A<sizeof(t1)> a; }
template <class T> void g(T t2) { A<sizeof(t2)> a; }
the two As are treated as equivalent; that's what the
comparing_specializations code is trying to prevent.
&& DECL_CONTEXT (t1) != DECL_CONTEXT (t2))
/* When comparing hash table entries, only an exact match is
good enough; we don't want to replace 'this' with the
version from another function. But be more flexible
with parameters with identical contexts. */
return false;
if (same_type_p (TREE_TYPE (t1), TREE_TYPE (t2)))
{
if (DECL_ARTIFICIAL (t1) ^ DECL_ARTIFICIAL (t2))
return false;
if (CONSTRAINT_VAR_P (t1) ^ CONSTRAINT_VAR_P (t2))
return false;
if (DECL_ARTIFICIAL (t1)
|| (DECL_PARM_LEVEL (t1) == DECL_PARM_LEVEL (t2)
&& DECL_PARM_INDEX (t1) == DECL_PARM_INDEX (t2)))
return true;
}
return false;
@@ -3974,63 +3975,68 @@ cp_tree_equal (tree t1, tree t2)
return true;
case CONSTRAINT_INFO:
return cp_tree_equal (CI_ASSOCIATED_CONSTRAINTS (t1),
CI_ASSOCIATED_CONSTRAINTS (t2));
case CHECK_CONSTR:
return (CHECK_CONSTR_CONCEPT (t1) == CHECK_CONSTR_CONCEPT (t2)
&& comp_template_args (CHECK_CONSTR_ARGS (t1),
CHECK_CONSTR_ARGS (t2)));
case TREE_VEC:
/* These are template args. Really we should be getting the
caller to do this as it knows it to be true. */
if (!comp_template_args (t1, t2, NULL, NULL, false))
return false;
return true;
case SIZEOF_EXPR:
case ALIGNOF_EXPR:
+ case NOEXCEPT_EXPR:
{
tree o1 = TREE_OPERAND (t1, 0);
tree o2 = TREE_OPERAND (t2, 0);
if (code1 == SIZEOF_EXPR)
{
if (SIZEOF_EXPR_TYPE_P (t1))
o1 = TREE_TYPE (o1);
if (SIZEOF_EXPR_TYPE_P (t2))
o2 = TREE_TYPE (o2);
}
- else if (ALIGNOF_EXPR_STD_P (t1) != ALIGNOF_EXPR_STD_P (t2))
+ else if (code1 == ALIGNOF_EXPR
+ && ALIGNOF_EXPR_STD_P (t1) != ALIGNOF_EXPR_STD_P (t2))
return false;
if (TREE_CODE (o1) != TREE_CODE (o2))
return false;
- if (ARGUMENT_PACK_P (o1))
- return template_args_equal (o1, o2);
- else if (TYPE_P (o1))
- return same_type_p (o1, o2);
- else
- return cp_tree_equal (o1, o2);
+ {
+ cp_unevaluated ev;
+ if (code1 == SIZEOF_EXPR && ARGUMENT_PACK_P (o1))
+ return template_args_equal (o1, o2);
+ else if (code1 != NOEXCEPT_EXPR && TYPE_P (o1))
Are these code1 checks necessary? I don't mind them, but I would have
expected the code to work without them.
+ return same_type_p (o1, o2);
+ else
+ return cp_tree_equal (o1, o2);
+ }
}
case MODOP_EXPR:
{
tree t1_op1, t2_op1;
if (!cp_tree_equal (TREE_OPERAND (t1, 0), TREE_OPERAND (t2, 0)))
return false;
t1_op1 = TREE_OPERAND (t1, 1);
t2_op1 = TREE_OPERAND (t2, 1);
if (TREE_CODE (t1_op1) != TREE_CODE (t2_op1))
return false;
return cp_tree_equal (TREE_OPERAND (t1, 2), TREE_OPERAND (t2, 2));
}
case PTRMEM_CST:
/* Two pointer-to-members are the same if they point to the same
field or function in the same class. */
diff --git a/gcc/testsuite/g++.dg/template/canon-type-15.C
b/gcc/testsuite/g++.dg/template/canon-type-15.C
new file mode 100644
index 00000000000..064e14c510d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/canon-type-15.C
@@ -0,0 +1,5 @@
+// { dg-do compile { target c++11 } }
+template<unsigned u> struct size_c{ static constexpr unsigned value = u; };
+template<class T> auto return_size(T t) -> size_c<sizeof(t)>;
+template<class T> auto return_size(T t) -> size_c<sizeof(t)>;
+static_assert(decltype(return_size('a'))::value == 1u, "");
diff --git a/gcc/testsuite/g++.dg/template/canon-type-16.C
b/gcc/testsuite/g++.dg/template/canon-type-16.C
new file mode 100644
index 00000000000..99361cbac30
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/canon-type-16.C
@@ -0,0 +1,6 @@
+// { dg-do compile { target c++11 } }
+template<bool u> struct bool_c{ static constexpr bool value = u; };
+template<class T> auto noexcepty(T t) -> bool_c<noexcept(t())>;
+template<class T> auto noexcepty(T t) -> bool_c<noexcept(t())>;
+struct foo { void operator()() noexcept; };
+static_assert(decltype(noexcepty(foo{}))::value, "");
diff --git a/gcc/testsuite/g++.dg/template/canon-type-17.C
b/gcc/testsuite/g++.dg/template/canon-type-17.C
new file mode 100644
index 00000000000..0555c8d0a42
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/canon-type-17.C
@@ -0,0 +1,5 @@
+// { dg-do compile { target c++11 } }
+template<unsigned u> struct size_c{ static constexpr unsigned value = u; };
+template<class... T> auto return_size(T... t) -> size_c<sizeof...(t)>;
+template<class... T> auto return_size(T... t) -> size_c<sizeof...(t)>;
+static_assert(decltype(return_size('a'))::value == 1u, "");
diff --git a/gcc/testsuite/g++.dg/template/canon-type-18.C
b/gcc/testsuite/g++.dg/template/canon-type-18.C
new file mode 100644
index 00000000000..2510181725c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/canon-type-18.C
@@ -0,0 +1,6 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wno-pedantic" }
+template<unsigned u> struct size_c{ static constexpr unsigned value = u; };
+template<class T> auto get_align(T t) -> size_c<alignof(t)>;
+template<class T> auto get_align(T t) -> size_c<alignof(t)>;
+static_assert(decltype(get_align('a'))::value == 1u, "");
