[C++ Patch / RFC] PR 50864

[C++ Patch / RFC] PR 50864

[Paolo Carlini]

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

Hi,

one more / RFC, for the ICE on invalid part of these issues with '->'.

The below tries to catch the problem very early, in 
cp_parser_postfix_dot_deref_expression and apparently works fine, passes 
the testsuite, etc. Is it too early? Is the check tight enough?

Thanks,
Paolo.

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

[-- Attachment #2: patch_50864_draft --]
[-- Type: text/plain, Size: 1164 bytes --]

Index: testsuite/g++.dg/template/crash109.C
===================================================================
--- testsuite/g++.dg/template/crash109.C	(revision 0)
+++ testsuite/g++.dg/template/crash109.C	(revision 0)
@@ -0,0 +1,10 @@
+// PR c++/50864
+
+namespace impl
+{
+  template <class T> T create();
+}
+
+template <class T, class U, __SIZE_TYPE__
+	  = sizeof(impl::create<T>() -> impl::create<U>())>  // { dg-error "not a class member" } 
+struct foo;
Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 180532)
+++ cp/parser.c	(working copy)
@@ -5673,6 +5673,15 @@ cp_parser_postfix_dot_deref_expression (cp_parser
 	{
 	  if (name != error_mark_node && !BASELINK_P (name) && parser->scope)
 	    {
+	      if (TREE_CODE (parser->scope) == NAMESPACE_DECL
+		  && TREE_CODE (postfix_expression) == ARROW_EXPR)
+		{
+		  error_at (token->location, "%<%D::%D%> is not a class member",
+			    parser->scope, name);
+		  parser->context->object_type = NULL_TREE;
+		  return error_mark_node;
+		}
+
 	      name = build_qualified_name (/*type=*/NULL_TREE,
 					   parser->scope,
 					   name,

Re: [C++ Patch / RFC] PR 50864

[Jason Merrill]

On 10/26/2011 04:04 PM, Paolo Carlini wrote:
> The below tries to catch the problem very early, in
> cp_parser_postfix_dot_deref_expression and apparently works fine, passes
> the testsuite, etc. Is it too early? Is the check tight enough?

At a glance, it looks too early; it's valid to have namespace-qualified 
names after ->.

namespace A
{
   struct B
   {
     int i;
   };
};

A::B* b;
int i = b->A::B::i;

Jason

Re: [C++ Patch / RFC] PR 50864

[Paolo Carlini]

Hi,
> At a glance, it looks too early; it's valid to have 
> namespace-qualified names after ->.
>
> namespace A
> {
>   struct B
>   {
>     int i;
>   };
> };
>
> A::B* b;
> int i = b->A::B::i;
I was also trying to construct such kind of example myself... but my 
patch does not regress on the testcase you wrote down. I can tell you 
exactly why, if you like..

Paolo.

Re: [C++ Patch / RFC] PR 50864

[Paolo Carlini]

On 10/26/2011 10:30 PM, Paolo Carlini wrote:
> Hi,
>> At a glance, it looks too early; it's valid to have 
>> namespace-qualified names after ->.
>>
>> namespace A
>> {
>>   struct B
>>   {
>>     int i;
>>   };
>> };
>>
>> A::B* b;
>> int i = b->A::B::i;
> I was also trying to construct such kind of example myself... but my 
> patch does not regress on the testcase you wrote down. I can tell you 
> exactly why, if you like..
We have that parser->scope is a RECORD_TYPE and postfix_expression is an 
INDIRECT_REF.

Paolo.

Re: [C++ Patch / RFC] PR 50864

[Paolo Carlini]

On 10/26/2011 10:35 PM, Paolo Carlini wrote:
> On 10/26/2011 10:30 PM, Paolo Carlini wrote:
>> Hi,
>>> At a glance, it looks too early; it's valid to have 
>>> namespace-qualified names after ->.
>>>
>>> namespace A
>>> {
>>>   struct B
>>>   {
>>>     int i;
>>>   };
>>> };
>>>
>>> A::B* b;
>>> int i = b->A::B::i;
>> I was also trying to construct such kind of example myself... but my 
>> patch does not regress on the testcase you wrote down. I can tell you 
>> exactly why, if you like..
> We have that parser->scope is a RECORD_TYPE and postfix_expression is 
> an INDIRECT_REF.
In this case, for example (like PR50870):

namespace impl
{
   struct inner
   {
     template <class T> T create();
   };
}

template <class T, class U, __SIZE_TYPE__
       = sizeof(impl::inner::create<T>() -> impl::inner::create<U>())>
struct foo;

we are also Ok, code is accepted, because name is a BASELINK and the new 
check isn't even reached (postfix_expression would be an ARROW_EXPR, but 
parser->scope again a RECORD_TYPE. More generally, in all the legal 
tests I tried by hand (outside the testsuite), when we get there 
parser->scope is always a RECORD_TYPE)

But if you feel more comfortable about performing the check elsewhere, I 
can try that of course.

Paolo.

Re: [C++ Patch / RFC] PR 50864

[Jason Merrill]

On 10/26/2011 04:35 PM, Paolo Carlini wrote:
> We have that parser->scope is a RECORD_TYPE and postfix_expression is an
> INDIRECT_REF.

Ah, OK.  I guess we swallow up the namespace while parsing the full 
nested-name-specifier and it isn't a problem.  So I think handling this 
here should be OK.  But why check for ARROW_EXPR?  Can't you construct a 
similar testcase using .?

I'll deal with 50870.

Jason

Re: [C++ Patch / RFC] PR 50864

[Paolo Carlini]

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

On 11/07/2011 10:45 PM, Jason Merrill wrote:
> On 10/26/2011 04:35 PM, Paolo Carlini wrote:
>> We have that parser->scope is a RECORD_TYPE and postfix_expression is an
>> INDIRECT_REF.
> Ah, OK.  I guess we swallow up the namespace while parsing the full 
> nested-name-specifier and it isn't a problem.  So I think handling 
> this here should be OK.  But why check for ARROW_EXPR?  Can't you 
> construct a similar testcase using .?
Ah, very good. I'll investigate that. In the meanwhile I'm applying the 
below when testing finishes, which indeed, cannot hurt, the code ends-up 
being a bit cleaner.
> I'll deal with 50870.
Excellent.

Paolo.

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

[-- Attachment #2: CL_50864_code --]
[-- Type: text/plain, Size: 149 bytes --]

2011-11-07  Paolo Carlini  <paolo.carlini@oracle.com>

	* pt.c (tsubst_copy_and_build): Fix qualified_name_lookup_error
	call in case COMPONENT_REF.

[-- Attachment #3: patch_50864_code --]
[-- Type: text/plain, Size: 1195 bytes --]

Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 180619)
+++ cp/pt.c	(working copy)
@@ -13741,14 +13741,12 @@ tsubst_copy_and_build (tree t,
 	else if (TREE_CODE (member) == SCOPE_REF
 		 && TREE_CODE (TREE_OPERAND (member, 1)) == TEMPLATE_ID_EXPR)
 	  {
-	    tree tmpl;
-	    tree args;
-
 	    /* Lookup the template functions now that we know what the
 	       scope is.  */
-	    tmpl = TREE_OPERAND (TREE_OPERAND (member, 1), 0);
-	    args = TREE_OPERAND (TREE_OPERAND (member, 1), 1);
-	    member = lookup_qualified_name (TREE_OPERAND (member, 0), tmpl,
+	    tree scope = TREE_OPERAND (member, 0);
+	    tree tmpl = TREE_OPERAND (TREE_OPERAND (member, 1), 0);
+	    tree args = TREE_OPERAND (TREE_OPERAND (member, 1), 1);
+	    member = lookup_qualified_name (scope, tmpl,
 					    /*is_type_p=*/false,
 					    /*complain=*/false);
 	    if (BASELINK_P (member))
@@ -13762,7 +13760,7 @@ tsubst_copy_and_build (tree t,
 	      }
 	    else
 	      {
-		qualified_name_lookup_error (object_type, tmpl, member,
+		qualified_name_lookup_error (scope, tmpl, member,
 					     input_location);
 		return error_mark_node;
 	      }