From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19261 invoked by alias); 3 Dec 2015 22:08:29 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 19242 invoked by uid 89); 3 Dec 2015 22:08:28 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=2.3 required=5.0 tests=AWL,BAYES_50,SPAM_SUBJECT1,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 03 Dec 2015 22:08:27 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id 7E46C351E61 for ; Thu, 3 Dec 2015 22:08:26 +0000 (UTC) Received: from [10.3.235.63] (vpn-235-63.phx2.redhat.com [10.3.235.63]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id tB3M8PEc015092; Thu, 3 Dec 2015 17:08:25 -0500 Message-ID: <1449180505.8490.36.camel@surprise> Subject: Re: [PATCH 07/10] Fix g++.dg/template/ref3.C From: David Malcolm To: Jason Merrill Cc: gcc-patches@gcc.gnu.org Date: Thu, 03 Dec 2015 22:08:00 -0000 In-Reply-To: <5660A843.60702@redhat.com> References: <565627A0.6040107@redhat.com> <1449154548-43964-1-git-send-email-dmalcolm@redhat.com> <1449154548-43964-8-git-send-email-dmalcolm@redhat.com> <5660A843.60702@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-12/txt/msg00500.txt.bz2 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 struct A {}; > > 4 > > 5 template struct B > > 6 { > > 7 A<(T)0> b; // { dg-error "constant|not a valid" } > > 8 A a; // { dg-error "constant|not a valid" } > > 9 }; > > 10 > > 11 B 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': > > 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': > > 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': > > 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 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=, nt=) at ../../src/gcc/cp/pt.c:7778 > > which calls: > > cp_tree_equal (t1=, t2=) 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? > > I'm not sure what the ideal fix for this is; for now I've worked > > around it by updating the dg directives to reflect the new output. > > > > gcc/testsuite/ChangeLog: > > * g++.dg/template/ref3.C: Update locations of dg directives. > > --- > > gcc/testsuite/g++.dg/template/ref3.C | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/gcc/testsuite/g++.dg/template/ref3.C b/gcc/testsuite/g++.dg/template/ref3.C > > index 976c093..6e568c3 100644 > > --- a/gcc/testsuite/g++.dg/template/ref3.C > > +++ b/gcc/testsuite/g++.dg/template/ref3.C > > @@ -4,8 +4,10 @@ template struct A {}; > > > > template struct B > > { > > - A<(T)0> b; // { dg-error "constant|not a valid" } > > - A a; // { dg-error "constant|not a valid" } > > + A<(T)0> b; // { dg-error "constant" "" { target c++98_only } } > > + // { dg-error "not a valid" "" { target c++11 } 7 } > > + > > + A a; // { dg-error "not a valid" "" { target c++11 } } > > }; > > > > B b; > > >