On Fri, 2015-12-04 at 11:01 -0500, Jason Merrill wrote:
> On 12/03/2015 05:08 PM, David Malcolm wrote:
> > On Thu, 2015-12-03 at 15:38 -0500, Jason Merrill wrote:
> >> On 12/03/2015 09:55 AM, David Malcolm wrote:
> >>> Testcase g++.dg/template/ref3.C:
> >>>
> >>> 1 // PR c++/28341
> >>> 2
> >>> 3 template<const int&> struct A {};
> >>> 4
> >>> 5 template<typename T> struct B
> >>> 6 {
> >>> 7 A<(T)0> b; // { dg-error "constant|not a valid" }
> >>> 8 A<T(0)> a; // { dg-error "constant|not a valid" }
> >>> 9 };
> >>> 10
> >>> 11 B<const int&> b;
> >>>
> >>> The output of this test for both c++11 and c++14 is unaffected
> >>> by the patch kit:
> >>> g++.dg/template/ref3.C: In instantiation of 'struct B<const int&>':
> >>> g++.dg/template/ref3.C:11:15: required from here
> >>> g++.dg/template/ref3.C:7:11: error: '0' is not a valid template
> >>> argument for type 'const int&' because it is not an lvalue
> >>> g++.dg/template/ref3.C:8:11: error: '0' is not a valid template
> >>> argument for type 'const int&' because it is not an lvalue
> >>>
> >>> However, the c++98 output is changed:
> >>>
> >>> Status quo for c++98:
> >>> g++.dg/template/ref3.C: In instantiation of 'struct B<const int&>':
> >>> g++.dg/template/ref3.C:11:15: required from here
> >>> g++.dg/template/ref3.C:7:11: error: a cast to a type other than an
> >>> integral or enumeration type cannot appear in a constant-expression
> >>> g++.dg/template/ref3.C:8:11: error: a cast to a type other than an
> >>> integral or enumeration type cannot appear in a constant-expression
> >>>
> >>> (line 7 and 8 are at the closing semicolon for fields b and a)
> >>>
> >>> With the patchkit for c++98:
> >>> g++.dg/template/ref3.C: In instantiation of 'struct B<const int&>':
> >>> g++.dg/template/ref3.C:11:15: required from here
> >>> g++.dg/template/ref3.C:7:5: error: a cast to a type other than an
> >>> integral or enumeration type cannot appear in a constant-expression
> >>> g++.dg/template/ref3.C:7:5: error: a cast to a type other than an
> >>> integral or enumeration type cannot appear in a constant-expression
> >>>
> >>> So the 2nd:
> >>> "error: a cast to a type other than an integral or enumeration type
> >>> cannot appear in a constant-expression"
> >>> moves from line 8 to line 7 (and moves them to earlier, having ranges)
> >>>
> >>> What's happening is that cp_parser_enclosed_template_argument_list
> >>> builds a CAST_EXPR, the first time from cp_parser_cast_expression,
> >>> the second time from cp_parser_functional_cast; these have locations
> >>> representing the correct respective caret&ranges, i.e.:
> >>>
> >>> A<(T)0> b;
> >>> ^~~~
> >>>
> >>> and:
> >>>
> >>> A<T(0)> a;
> >>> ^~~~
> >>>
> >>> Eventually finish_template_type is called for each, to build a
> >>> RECORD_TYPE,
> >>> and we get a cache hit the 2nd time through here in pt.c:
> >>> 8281 hash = spec_hasher::hash (&elt);
> >>> 8282 entry = type_specializations->find_with_hash (&elt, hash);
> >>> 8283
> >>> 8284 if (entry)
> >>> 8285 return entry->spec;
> >>>
> >>> due to:
> >>> template_args_equal (ot=<cast_expr 0x7ffff19bc400>, nt=<cast_expr
> >>> 0x7ffff19bc480>) at ../../src/gcc/cp/pt.c:7778
> >>> which calls:
> >>> cp_tree_equal (t1=<cast_expr 0x7ffff19bc400>, t2=<cast_expr
> >>> 0x7ffff19bc480>) at ../../src/gcc/cp/tree.c:2833
> >>> and returns equality.
> >>>
> >>> Hence we get a single RECORD_TYPE for the type A<(T)(0)>, and hence
> >>> when issuing the errors it uses the TREE_VEC for the first one,
> >>> using the location of the first line.
> >>
> >> Why does the type sharing affect where the parser gives the error?
> >
> > I believe what's happening is that the patchkit is setting location_t
> > values for more expressions than before, including the expression for
> > the template param. pt.c:tsubst_expr has this:
> >
> > if (EXPR_HAS_LOCATION (t))
> > input_location = EXPR_LOCATION (t);
> >
> > I believe that before (in the status quo), the substituted types didn't
> > have location_t values, and hence the above conditional didn't fire;
> > input_location was coming from a *token* where the expansion happened,
> > hence we got an error message on the relevant line for each expansion.
> >
> > With the patch, the substituted types have location_t values within
> > their params, hence the conditional above fires: input_location is
> > updated to use the EXPR_LOCATION, which comes from that of the param
> > within the type - but with type-sharing it's using the first place where
> > the type is created.
> >
> > Perhaps a better fix is for cp_parser_non_integral_constant_expression
> > to take a location_t, rather than have it rely on input_location?
>
> Ah, I see, the error is coming from tsubst_copy_and_build, not
> cp_parser_non_integral_constant_expression. So indeed this is an effect
> of the canonicalization of template instances, and we aren't going to
> fix it in the context of this patchset. But this is still a bug, so I'd
> rather have an xfail and a PR than change the expected output.
Is the following what you had in mind?
gcc/testsuite/ChangeLog:
* g++.dg/template/ref3.C: Add XFAIL (PR c++/68699).
---
gcc/testsuite/g++.dg/template/ref3.C | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/gcc/testsuite/g++.dg/template/ref3.C
b/gcc/testsuite/g++.dg/template/ref3.C
index 976c093..91e3c93 100644
--- a/gcc/testsuite/g++.dg/template/ref3.C
+++ b/gcc/testsuite/g++.dg/template/ref3.C
@@ -5,7 +5,8 @@ template<const int&> struct A {};
template<typename T> struct B
{
A<(T)0> b; // { dg-error "constant|not a valid" }
- A<T(0)> a; // { dg-error "constant|not a valid" }
+ A<T(0)> a; // { dg-error "constant|not a valid" "" { xfail c++98_only } }
+ // PR c++/68699
};
B<const int&> b;
--
1.8.5.3
