[C++ Patch] PR 84092 ("[8 Regression] ICE on C++14 code with variable template: in build_qualified_name, at cp/tree.c:2043")

[C++ Patch] PR 84092 ("[8 Regression] ICE on C++14 code with variable template: in build_qualified_name, at cp/tree.c:2043")

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 720 bytes --]

Hi,

the fix for c++/81236 removed some special code for dependent_p from 
finish_id_expression, and now finish_qualified_id_expr is used for this 
snippet too. Then special code at the beginning of the latter takes care 
of calling build_qualified_name to create the relevant SCOPE_REF. 
Therefore it seems to me that - unless we really want to return an 
OFFSET_REF - at that point we are done, we don't want to get to the end 
of the following long conditional and call again build_qualified_name on 
the SCOPE_REF and ICE. We don't need convert_from_reference either 
because build_qualified_name is passed a null type. Finishing testing 
(in the library) on x86_64-linux.

Thanks! Paolo.

////////////////////////


[-- Attachment #2: CL_84092 --]
[-- Type: text/plain, Size: 294 bytes --]

/cp
2018-01-29  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/84092
	* semantics.c (finish_qualified_id_expr): Maybe early return when handling
	an UNBOUND_CLASS_TEMPLATE.

/testsuite
2018-01-29  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/84092
	* g++.dg/cpp1y/var-templ57.C: New.

[-- Attachment #3: patch_84092 --]
[-- Type: text/plain, Size: 1309 bytes --]

Index: cp/semantics.c
===================================================================
--- cp/semantics.c	(revision 257139)
+++ cp/semantics.c	(working copy)
@@ -2001,12 +2001,16 @@ finish_qualified_id_expr (tree qualifying_class,
   if (template_p)
     {
       if (TREE_CODE (expr) == UNBOUND_CLASS_TEMPLATE)
-	/* cp_parser_lookup_name thought we were looking for a type,
-	   but we're actually looking for a declaration.  */
-	expr = build_qualified_name (/*type*/NULL_TREE,
-				     TYPE_CONTEXT (expr),
-				     TYPE_IDENTIFIER (expr),
-				     /*template_p*/true);
+	{
+	  /* cp_parser_lookup_name thought we were looking for a type,
+	     but we're actually looking for a declaration.  */
+	  expr = build_qualified_name (/*type*/NULL_TREE,
+				       TYPE_CONTEXT (expr),
+				       TYPE_IDENTIFIER (expr),
+				       /*template_p*/true);
+	  if (!(address_p && done))
+	    return expr;
+	}
       else
 	check_template_keyword (expr);
     }
Index: testsuite/g++.dg/cpp1y/var-templ57.C
===================================================================
--- testsuite/g++.dg/cpp1y/var-templ57.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/var-templ57.C	(working copy)
@@ -0,0 +1,4 @@
+// PR c++/84092
+// { dg-do compile { target c++14 } }
+
+template < typename T > int a (T::template b);

Re: [C++ Patch] PR 84092 ("[8 Regression] ICE on C++14 code with variable template: in build_qualified_name, at cp/tree.c:2043")

[Jason Merrill]

On Mon, Jan 29, 2018 at 10:45 AM, Paolo Carlini
<paolo.carlini@oracle.com> wrote:
> the fix for c++/81236 removed some special code for dependent_p from
> finish_id_expression, and now finish_qualified_id_expr is used for this
> snippet too. Then special code at the beginning of the latter takes care of
> calling build_qualified_name to create the relevant SCOPE_REF. Therefore it
> seems to me that - unless we really want to return an OFFSET_REF - at that
> point we are done, we don't want to get to the end of the following long
> conditional and call again build_qualified_name on the SCOPE_REF and ICE. We
> don't need convert_from_reference either because build_qualified_name is
> passed a null type. Finishing testing (in the library) on x86_64-linux.

Hmm, it seems to me that the later code would handle this case fine
if, instead of calling build_qualified_name here, we do

qualifying_class = TYPE_CONTEXT (expr);
expr = TYPE_IDENTIFIER (expr);

Does that work?

Jason

Re: [C++ Patch] PR 84092 ("[8 Regression] ICE on C++14 code with variable template: in build_qualified_name, at cp/tree.c:2043")

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 1250 bytes --]

Hi,

On 29/01/2018 21:41, Jason Merrill wrote:
> On Mon, Jan 29, 2018 at 10:45 AM, Paolo Carlini
> <paolo.carlini@oracle.com> wrote:
>> the fix for c++/81236 removed some special code for dependent_p from
>> finish_id_expression, and now finish_qualified_id_expr is used for this
>> snippet too. Then special code at the beginning of the latter takes care of
>> calling build_qualified_name to create the relevant SCOPE_REF. Therefore it
>> seems to me that - unless we really want to return an OFFSET_REF - at that
>> point we are done, we don't want to get to the end of the following long
>> conditional and call again build_qualified_name on the SCOPE_REF and ICE. We
>> don't need convert_from_reference either because build_qualified_name is
>> passed a null type. Finishing testing (in the library) on x86_64-linux.
> Hmm, it seems to me that the later code would handle this case fine
> if, instead of calling build_qualified_name here, we do
>
> qualifying_class = TYPE_CONTEXT (expr);
> expr = TYPE_IDENTIFIER (expr);
>
> Does that work?
Indeed it appears to work great and would make for a nice 
simplification, thanks! Testing the below is already past the C++ 
testsuite. Ok if it fully passes?

Thanks again,
Paolo.

//////////////////

[-- Attachment #2: patch_84092_2 --]
[-- Type: text/plain, Size: 1185 bytes --]

Index: cp/semantics.c
===================================================================
--- cp/semantics.c	(revision 257159)
+++ cp/semantics.c	(working copy)
@@ -2001,12 +2001,12 @@ finish_qualified_id_expr (tree qualifying_class,
   if (template_p)
     {
       if (TREE_CODE (expr) == UNBOUND_CLASS_TEMPLATE)
-	/* cp_parser_lookup_name thought we were looking for a type,
-	   but we're actually looking for a declaration.  */
-	expr = build_qualified_name (/*type*/NULL_TREE,
-				     TYPE_CONTEXT (expr),
-				     TYPE_IDENTIFIER (expr),
-				     /*template_p*/true);
+	{
+	  /* cp_parser_lookup_name thought we were looking for a type,
+	     but we're actually looking for a declaration.  */
+	  qualifying_class = TYPE_CONTEXT (expr);
+	  expr = TYPE_IDENTIFIER (expr);
+	}
       else
 	check_template_keyword (expr);
     }
Index: testsuite/g++.dg/cpp1y/var-templ57.C
===================================================================
--- testsuite/g++.dg/cpp1y/var-templ57.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/var-templ57.C	(working copy)
@@ -0,0 +1,4 @@
+// PR c++/84092
+// { dg-do compile { target c++14 } }
+
+template < typename T > int a (T::template b);

Re: [C++ Patch] PR 84092 ("[8 Regression] ICE on C++14 code with variable template: in build_qualified_name, at cp/tree.c:2043")

[Jason Merrill]

OK.

On Mon, Jan 29, 2018 at 5:00 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
> On 29/01/2018 21:41, Jason Merrill wrote:
>>
>> On Mon, Jan 29, 2018 at 10:45 AM, Paolo Carlini
>> <paolo.carlini@oracle.com> wrote:
>>>
>>> the fix for c++/81236 removed some special code for dependent_p from
>>> finish_id_expression, and now finish_qualified_id_expr is used for this
>>> snippet too. Then special code at the beginning of the latter takes care
>>> of
>>> calling build_qualified_name to create the relevant SCOPE_REF. Therefore
>>> it
>>> seems to me that - unless we really want to return an OFFSET_REF - at
>>> that
>>> point we are done, we don't want to get to the end of the following long
>>> conditional and call again build_qualified_name on the SCOPE_REF and ICE.
>>> We
>>> don't need convert_from_reference either because build_qualified_name is
>>> passed a null type. Finishing testing (in the library) on x86_64-linux.
>>
>> Hmm, it seems to me that the later code would handle this case fine
>> if, instead of calling build_qualified_name here, we do
>>
>> qualifying_class = TYPE_CONTEXT (expr);
>> expr = TYPE_IDENTIFIER (expr);
>>
>> Does that work?
>
> Indeed it appears to work great and would make for a nice simplification,
> thanks! Testing the below is already past the C++ testsuite. Ok if it fully
> passes?
>
> Thanks again,
> Paolo.
>
> //////////////////