C++ Patch for c++/60894

C++ Patch for c++/60894

[Fabien Chêne]

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

Hi,

The problem here is that the use of an elaborated-type-specifier
(introduced via 'struct') does not find USING_DECLs, whereas it does
for a simple-type-specifier. That's caused by xref_tag (through
lookup_and_check_tag) that does not strip USING_DECLs and thus fails
to return a type when it happens to be the target of the USING_DECL.

Unfortunately, just stripping the USING_DECL in lookup_and_check_tag
does not really work because some diagnotic codes expect the
USING_DECL not to be stripped.
Consequently, I have fixed it by introcucing a new parameter to
xref_tag whose purpose is to know if stripping USING_DECLs is
desirable. Not very exciting to fix it this way, but that is the only
solution I managed to find.

Tested x86_64 linux without regressions. OK to commit to trunk ? And
branches after some weeks of incubation ?

gcc/cp/ChangeLog

2014-09-24  Fabien Chêne  <fabien@gcc.gnu.org>

    * cp-tree.h (xref_tag): Add a new bool parameter.
    * parser.c (cp_parser_elaborated_type_specifier): Call xref_tag
    with strip_using_p set to true.
    (cp_parser_class_head): Call xref_tag with strip_using_p set to
    false.
    * rtti.c (init_rtti_processing): Likewise.
    (build_dynamic_cast_1): Likewise.
    (tinfo_base_init): Likewise.
    (emit_support_tinfos): Likewise.
    * lambda.c (begin_lambda_type): Likewise.
    (vla_capture_type): Likewise.
    * decl.c (lookup_and_check_tag): Add a new bool parameter. Strip
    the USING_DECL if strip_using_p is set. Declare the variable t at
    the use.
    (xref_tag_1): Add a new bool parameter. Forward strip_using_p to
    lookup_and_check_tag.
    (xref_tag): Add a new bool parameter. Forward strip_using_p to
    xref_tag_1.
    (xref_tag_from_type): Call xref_tag with strip_using_p set to
    false.
    (start_enum): Call lookup_and_check_tag with strip_using_p set to
    false. Call xref_tag with strip_using_p set to false.

gcc/testsuite/ChangeLog

2014-09-24  Fabien Chêne  <fabien@gcc.gnu.org>

    PR c++/56243
    * g++.dg/lookup/using56.C: New.

-- 
Fabien

[-- Attachment #2: pr60894.patch --]
[-- Type: text/x-diff, Size: 8363 bytes --]

Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(révision 209870)
+++ gcc/cp/parser.c	(copie de travail)
@@ -15202,7 +15202,8 @@ cp_parser_elaborated_type_specifier (cp_
 						    token->location,
 						    /*declarator=*/NULL))
 	    return error_mark_node;
-	  type = xref_tag (tag_type, identifier, ts, template_p);
+	  type = xref_tag (tag_type, identifier, ts, template_p,
+			   /*strip_using_p=*/true);
 	}
     }
 
@@ -19930,7 +19931,7 @@ cp_parser_class_head (cp_parser* parser,
       if (!id)
 	id = make_anon_name ();
       type = xref_tag (class_key, id, /*tag_scope=*/ts_current,
-		       parser->num_template_parameter_lists);
+		       parser->num_template_parameter_lists, false);
     }
 
   /* Indicate whether this class was declared as a `class' or as a
Index: gcc/cp/cp-tree.h
===================================================================
--- gcc/cp/cp-tree.h	(révision 209870)
+++ gcc/cp/cp-tree.h	(copie de travail)
@@ -5245,7 +5245,8 @@ extern tree get_scope_of_declarator		(co
 extern void grok_special_member_properties	(tree);
 extern int grok_ctor_properties			(const_tree, const_tree);
 extern bool grok_op_properties			(tree, bool);
-extern tree xref_tag				(enum tag_types, tree, tag_scope, bool);
+extern tree xref_tag				(enum tag_types, tree, tag_scope,
+						 bool, bool);
 extern tree xref_tag_from_type			(tree, tree, tag_scope);
 extern bool xref_basetypes			(tree, tree);
 extern tree start_enum				(tree, tree, tree, bool, bool *);
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(révision 209870)
+++ gcc/cp/decl.c	(copie de travail)
@@ -92,7 +92,7 @@ static int typename_compare (const void
 static tree local_variable_p_walkfn (tree *, int *, void *);
 static tree record_builtin_java_type (const char *, int);
 static const char *tag_name (enum tag_types);
-static tree lookup_and_check_tag (enum tag_types, tree, tag_scope, bool);
+static tree lookup_and_check_tag (enum tag_types, tree, tag_scope, bool, bool);
 static int walk_namespaces_r (tree, walk_namespaces_fn, void *);
 static void maybe_deduce_size_from_array_init (tree, tree);
 static void layout_var_decl (tree);
@@ -11944,9 +11944,9 @@ check_elaborated_type_specifier (enum ta
 
 static tree
 lookup_and_check_tag (enum tag_types tag_code, tree name,
-		      tag_scope scope, bool template_header_p)
+		      tag_scope scope, bool template_header_p,
+		      bool strip_using_p)
 {
-  tree t;
   tree decl;
   if (scope == ts_global)
     {
@@ -11973,6 +11973,9 @@ lookup_and_check_tag (enum tag_types tag
 	      && DECL_TEMPLATE_TEMPLATE_PARM_P (decl))))
     decl = DECL_TEMPLATE_RESULT (decl);
 
+  if (strip_using_p)
+    decl = strip_using_decl (decl);
+
   if (decl && TREE_CODE (decl) == TYPE_DECL)
     {
       /* Look for invalid nested type:
@@ -12003,10 +12006,10 @@ lookup_and_check_tag (enum tag_types tag
 	     class C *c2;		// DECL_SELF_REFERENCE_P is true
 	   };  */
 
-      t = check_elaborated_type_specifier (tag_code,
-					   decl,
-					   template_header_p
-					   | DECL_SELF_REFERENCE_P (decl));
+      tree t = check_elaborated_type_specifier (tag_code,
+						decl,
+						template_header_p
+						| DECL_SELF_REFERENCE_P (decl));
       return t;
     }
   else if (decl && TREE_CODE (decl) == TREE_LIST)
@@ -12036,7 +12039,8 @@ lookup_and_check_tag (enum tag_types tag
 
 static tree
 xref_tag_1 (enum tag_types tag_code, tree name,
-            tag_scope orig_scope, bool template_header_p)
+            tag_scope orig_scope, bool template_header_p,
+	    bool strip_using_p)
 {
   enum tree_code code;
   tree t;
@@ -12072,7 +12076,8 @@ xref_tag_1 (enum tag_types tag_code, tre
     t = NULL_TREE;
   else
     t = lookup_and_check_tag  (tag_code, name,
-			       scope, template_header_p);
+			       scope, template_header_p,
+			       strip_using_p);
 
   if (t == error_mark_node)
     return error_mark_node;
@@ -12184,12 +12189,14 @@ xref_tag_1 (enum tag_types tag_code, tre
 
 tree
 xref_tag (enum tag_types tag_code, tree name,
-          tag_scope scope, bool template_header_p)
+          tag_scope scope, bool template_header_p,
+	  bool strip_using_p)
 {
   tree ret;
   bool subtime;
   subtime = timevar_cond_start (TV_NAME_LOOKUP);
-  ret = xref_tag_1 (tag_code, name, scope, template_header_p);
+  ret = xref_tag_1 (tag_code, name, scope, template_header_p,
+		    strip_using_p);
   timevar_cond_stop (TV_NAME_LOOKUP, subtime);
   return ret;
 }
@@ -12208,7 +12215,7 @@ xref_tag_from_type (tree old, tree id, t
   if (id == NULL_TREE)
     id = TYPE_IDENTIFIER (old);
 
-  return xref_tag (tag_kind, id, scope, false);
+  return xref_tag (tag_kind, id, scope, false, false);
 }
 
 /* Create the binfo hierarchy for REF with (possibly NULL) base list
@@ -12490,7 +12497,8 @@ start_enum (tree name, tree enumtype, tr
   if (!enumtype)
     enumtype = lookup_and_check_tag (enum_type, name,
 				     /*tag_scope=*/ts_current,
-				     /*template_header_p=*/false);
+				     /*template_header_p=*/false,
+				     /*strip_using_p=*/false);
 
   /* In case of a template_decl, the only check that should be deferred
      to instantiation time is the comparison of underlying types.  */
@@ -12562,7 +12570,7 @@ start_enum (tree name, tree enumtype, tr
 	}
       else
 	  enumtype = xref_tag (enum_type, name, /*tag_scope=*/ts_current,
-			       false);
+			       false, false);
 
       if (enumtype == error_mark_node)
 	return error_mark_node;
Index: gcc/cp/lambda.c
===================================================================
--- gcc/cp/lambda.c	(révision 209870)
+++ gcc/cp/lambda.c	(copie de travail)
@@ -138,7 +138,8 @@ begin_lambda_type (tree lambda)
     type = xref_tag (/*tag_code=*/record_type,
                      name,
                      /*scope=*/ts_lambda,
-                     /*template_header_p=*/false);
+                     /*template_header_p=*/false,
+		     /*strip_using_p=*/false);
     if (type == error_mark_node)
       return error_mark_node;
   }
@@ -416,7 +417,7 @@ static tree
 vla_capture_type (tree array_type)
 {
   static tree ptr_id, max_id;
-  tree type = xref_tag (record_type, make_anon_name (), ts_current, false);
+  tree type = xref_tag (record_type, make_anon_name (), ts_current, false, false);
   xref_basetypes (type, NULL_TREE);
   type = begin_class_definition (type);
   if (!ptr_id)
Index: gcc/cp/rtti.c
===================================================================
--- gcc/cp/rtti.c	(révision 209870)
+++ gcc/cp/rtti.c	(copie de travail)
@@ -152,7 +152,7 @@ init_rtti_processing (void)
 
   push_namespace (std_identifier);
   type_info_type = xref_tag (class_type, get_identifier ("type_info"),
-			     /*tag_scope=*/ts_current, false);
+			     /*tag_scope=*/ts_current, false, false);
   pop_namespace ();
   const_type_info_type_node
     = cp_build_qualified_type (type_info_type, TYPE_QUAL_CONST);
@@ -731,7 +731,7 @@ build_dynamic_cast_1 (tree type, tree ex
 	      push_abi_namespace ();
 	      tinfo_ptr = xref_tag (class_type,
 				    get_identifier ("__class_type_info"),
-				    /*tag_scope=*/ts_current, false);
+				    /*tag_scope=*/ts_current, false, false);
 
 	      tinfo_ptr = build_pointer_type
 		(cp_build_qualified_type
@@ -913,7 +913,7 @@ tinfo_base_init (tinfo_s *ti, tree targe
       tree real_type;
       push_abi_namespace ();
       real_type = xref_tag (class_type, ti->name,
-			    /*tag_scope=*/ts_current, false);
+			    /*tag_scope=*/ts_current, false, false);
       pop_abi_namespace ();
 
       if (!COMPLETE_TYPE_P (real_type))
@@ -1536,7 +1536,7 @@ emit_support_tinfos (void)
   push_abi_namespace ();
   bltn_type = xref_tag (class_type,
 			get_identifier ("__fundamental_type_info"),
-			/*tag_scope=*/ts_current, false);
+			/*tag_scope=*/ts_current, false, false);
   pop_abi_namespace ();
   if (!COMPLETE_TYPE_P (bltn_type))
     return;
Index: gcc/testsuite/g++.dg/lookup/using54.C
===================================================================
--- gcc/testsuite/g++.dg/lookup/using54.C	(révision 0)
+++ gcc/testsuite/g++.dg/lookup/using54.C	(révision 0)
@@ -0,0 +1,16 @@
+// PR c++/60894
+
+struct B
+{
+  struct S {};
+};
+
+struct D : B
+{
+  using B::S;
+  void doIt(struct S&);
+};
+
+void D::doIt(struct S&)
+{
+}

Re: C++ Patch for c++/60894

[Jason Merrill]

On 09/24/2014 05:06 PM, Fabien Chêne wrote:
> Unfortunately, just stripping the USING_DECL in lookup_and_check_tag
> does not really work because some diagnotic codes expect the
> USING_DECL not to be stripped.

How so?

Jason

Re: C++ Patch for c++/60894

[Jason Merrill]

On 09/24/2014 05:15 PM, Jason Merrill wrote:
> On 09/24/2014 05:06 PM, Fabien Chêne wrote:
>> Unfortunately, just stripping the USING_DECL in lookup_and_check_tag
>> does not really work because some diagnotic codes expect the
>> USING_DECL not to be stripped.

It seems to me that the problem is that lookup_and_check_tag is 
rejecting a USING_DECL rather than returning it.  What if we return the 
USING_DECL?

Jason

Re: C++ Patch for c++/60894

[Fabien Chêne]

2014-10-07 23:13 GMT+02:00 Jason Merrill <jason@redhat.com>:
> On 09/24/2014 05:15 PM, Jason Merrill wrote:
>>
>> On 09/24/2014 05:06 PM, Fabien Chêne wrote:
>>>
>>> Unfortunately, just stripping the USING_DECL in lookup_and_check_tag
>>> does not really work because some diagnotic codes expect the
>>> USING_DECL not to be stripped.
>
> It seems to me that the problem is that lookup_and_check_tag is rejecting a
> USING_DECL rather than returning it.  What if we return the USING_DECL?

If the USING_DECL is returned, the code below will be rejected as
expected, but the error message will not mention the line where the
USING_DECL appears as the previous definition, but at the target
declaration of the USING_DECL instead.

struct J
{
  struct type {};
};

struct L : J
{
  using J::type;
  struct type {};
};

The code doing that is in cp_parser_class_head, after the call to
xref_tag, at this point:

if (type != error_mark_node && COMPLETE_TYPE_P (type))
    {
      error_at (type_start_token->location, "redefinition of %q#T",
        type);
      error_at (type_start_token->location, "previous definition of %q+#T",
        type);
...

Actually, if xref_tag strips the USING_DECL, it finds a type already
complete and the original decl is lost for the diagnostic.
Trying to skip this error does not work because 'type' is really
expected not to be complete.
Hence, I guess the solution, however disgracious it could be,  is to
ignore USING_DECLS from cp_parser_class_head through xref_tag, and
wait for the appropriate diagnostic at finish_struct (in
supplement_binding more precisely).

-- 
Fabien

Re: C++ Patch for c++/60894

[Jason Merrill]

On 10/08/2014 01:30 PM, Fabien Chêne wrote:
> 2014-10-07 23:13 GMT+02:00 Jason Merrill <jason@redhat.com>:
>> It seems to me that the problem is that lookup_and_check_tag is rejecting a
>> USING_DECL rather than returning it.  What if we return the USING_DECL?
>
> If the USING_DECL is returned, the code below will be rejected as
> expected, but the error message will not mention the line where the
> USING_DECL appears as the previous definition, but at the target
> declaration of the USING_DECL instead.

I think that's what happens if you strip the USING_DECL and return what 
it points to; if you return the USING_DECL itself that shouldn't happen 
(though then the caller needs to be able to deal with getting a USING_DECL).

Jason

Re: C++ Patch for c++/60894

[Fabien Chêne]

2014-10-09 15:34 GMT+02:00 Jason Merrill <jason@redhat.com>:
[...]
>> If the USING_DECL is returned, the code below will be rejected as
>> expected, but the error message will not mention the line where the
>> USING_DECL appears as the previous definition, but at the target
>> declaration of the USING_DECL instead.
>
>
> I think that's what happens if you strip the USING_DECL and return what it
> points to; if you return the USING_DECL itself that shouldn't happen (though
> then the caller needs to be able to deal with getting a USING_DECL).

[Sorry for the delay] Humm, l_a_c_t returns a TYPE upon success, shall
I change it and return a DECL instead ?

-- 
Fabien

Re: C++ Patch for c++/60894

[Fabien Chêne]

2014-11-03 21:18 GMT+01:00 Fabien Chêne <fabien.chene@gmail.com>:
> 2014-10-09 15:34 GMT+02:00 Jason Merrill <jason@redhat.com>:
> [...]
>>> If the USING_DECL is returned, the code below will be rejected as
>>> expected, but the error message will not mention the line where the
>>> USING_DECL appears as the previous definition, but at the target
>>> declaration of the USING_DECL instead.
>>
>>
>> I think that's what happens if you strip the USING_DECL and return what it
>> points to; if you return the USING_DECL itself that shouldn't happen (though
>> then the caller needs to be able to deal with getting a USING_DECL).
>
> [Sorry for the delay] Humm, l_a_c_t returns a TYPE upon success, shall
> I change it and return a DECL instead ?

Ping. Before I made the change, I'd like to be sure this is what you
have in mind.
And sorry, my time is very limited at the moment...

-- 
Fabien

Re: C++ Patch for c++/60894

[Jason Merrill]

On 12/01/2014 07:01 AM, Fabien Chêne wrote:
> 2014-11-03 21:18 GMT+01:00 Fabien Chêne <fabien.chene@gmail.com>:
>> 2014-10-09 15:34 GMT+02:00 Jason Merrill <jason@redhat.com>:
>> [...]
>>>> If the USING_DECL is returned, the code below will be rejected as
>>>> expected, but the error message will not mention the line where the
>>>> USING_DECL appears as the previous definition, but at the target
>>>> declaration of the USING_DECL instead.
>>>
>>>
>>> I think that's what happens if you strip the USING_DECL and return what it
>>> points to; if you return the USING_DECL itself that shouldn't happen (though
>>> then the caller needs to be able to deal with getting a USING_DECL).
>>
>> [Sorry for the delay] Humm, l_a_c_t returns a TYPE upon success, shall
>> I change it and return a DECL instead ?
>
> Ping. Before I made the change, I'd like to be sure this is what you
> have in mind.

I think that makes sense.

Jason

Re: C++ Patch for c++/60894

[Paolo Carlini]

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

Hi,

On 12/01/2014 09:59 PM, Jason Merrill wrote:
> On 12/01/2014 07:01 AM, Fabien Chêne wrote:
>> 2014-11-03 21:18 GMT+01:00 Fabien Chêne <fabien.chene@gmail.com>:
>>> 2014-10-09 15:34 GMT+02:00 Jason Merrill <jason@redhat.com>:
>>> [...]
>>>>> If the USING_DECL is returned, the code below will be rejected as
>>>>> expected, but the error message will not mention the line where the
>>>>> USING_DECL appears as the previous definition, but at the target
>>>>> declaration of the USING_DECL instead.
>>>>
>>>>
>>>> I think that's what happens if you strip the USING_DECL and return 
>>>> what it
>>>> points to; if you return the USING_DECL itself that shouldn't 
>>>> happen (though
>>>> then the caller needs to be able to deal with getting a USING_DECL).
>>>
>>> [Sorry for the delay] Humm, l_a_c_t returns a TYPE upon success, shall
>>> I change it and return a DECL instead ?
>>
>> Ping. Before I made the change, I'd like to be sure this is what you
>> have in mind.
>
> I think that makes sense.

Today I was having a look to this pending issue and went astray due to 
the broken thread: I wondered if, basing on Fabien' first try and the 
comments accompanying tag_scope, it would make sense to use 
strip_using_decl only when the scope is ts_global (or maybe != 
ts_current)?!? The below certainly passes testing. Unless of course 
Fabien has the above implemented and ready (I don't, I miss the correct 
changes to the lookup_and_check_tag callers)

Thanks,
Paolo.

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

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 465 bytes --]

Index: decl.c
===================================================================
--- decl.c	(revision 220611)
+++ decl.c	(working copy)
@@ -12207,6 +12207,9 @@ lookup_and_check_tag (enum tag_types tag_code, tre
 	      && DECL_TEMPLATE_TEMPLATE_PARM_P (decl))))
     decl = DECL_TEMPLATE_RESULT (decl);
 
+  if (scope == ts_global)
+    decl = strip_using_decl (decl);
+
   if (decl && TREE_CODE (decl) == TYPE_DECL)
     {
       /* Look for invalid nested type:

Re: C++ Patch for c++/60894

[Jason Merrill]

On 02/11/2015 02:10 PM, Paolo Carlini wrote:
> Today I was having a look to this pending issue and went astray due to
> the broken thread: I wondered if, basing on Fabien' first try and the
> comments accompanying tag_scope, it would make sense to use
> strip_using_decl only when the scope is ts_global (or maybe !=
> ts_current)?!?

Good thought, that makes sense and is a lot simpler.  But I think since 
we have a ts_global block earlier in the function, let's do the 
stripping there, right after the call to lookup_name_prefer_type.  OK 
with that change.

Jason

Re: C++ Patch for c++/60894

[Paolo Carlini]

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

Hi,

On 02/13/2015 04:03 PM, Jason Merrill wrote:
> On 02/11/2015 02:10 PM, Paolo Carlini wrote:
>> Today I was having a look to this pending issue and went astray due to
>> the broken thread: I wondered if, basing on Fabien' first try and the
>> comments accompanying tag_scope, it would make sense to use
>> strip_using_decl only when the scope is ts_global (or maybe !=
>> ts_current)?!?
>
> Good thought, that makes sense and is a lot simpler.  But I think 
> since we have a ts_global block earlier in the function, let's do the 
> stripping there, right after the call to lookup_name_prefer_type.  OK 
> with that change.
Thanks. Thus I'm going to resolve the issue at least in mainline and 
4_9-branch with the below.

While working on this issue I noticed that we have another regression, 
which appeared in 4_7, having to do with using declarations but not 
involving redeclarations or redundant 'struct' in parameters:

struct B
{
   template<typename T>
   struct S {};
};

struct D : B
{
   using B::S;

   template<typename T>
   void doIt(/*struct*/ S<T>&);
};

Anything you would recommend besides filing a new Bug Report (or marking 
an existing one as regression)?!?

Thanks,
Paolo.

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

[-- Attachment #2: CL_60894 --]
[-- Type: text/plain, Size: 243 bytes --]

/cp
2015-02-13  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/60894
	* decl.c (lookup_and_check_tag): Use strip_using_decl.

/testsuite
2015-02-13  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/60894
	* g++.dg/lookup/using54.C: New.

[-- Attachment #3: patch_60894 --]
[-- Type: text/plain, Size: 990 bytes --]

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 220688)
+++ cp/decl.c	(working copy)
@@ -12205,6 +12205,7 @@ lookup_and_check_tag (enum tag_types tag_code, tre
       /* First try ordinary name lookup, ignoring hidden class name
 	 injected via friend declaration.  */
       decl = lookup_name_prefer_type (name, 2);
+      decl = strip_using_decl (decl);
       /* If that fails, the name will be placed in the smallest
 	 non-class, non-function-prototype scope according to 3.3.1/5.
 	 We may already have a hidden name declared as friend in this
Index: testsuite/g++.dg/lookup/using54.C
===================================================================
--- testsuite/g++.dg/lookup/using54.C	(revision 0)
+++ testsuite/g++.dg/lookup/using54.C	(working copy)
@@ -0,0 +1,16 @@
+// PR c++/60894
+
+struct B
+{
+  struct S {};
+};
+
+struct D : B
+{
+  using B::S;
+  void doIt(struct S&);
+};
+
+void D::doIt(struct S&)
+{
+}

Re: C++ Patch for c++/60894

[Jason Merrill]

On 02/13/2015 01:56 PM, Paolo Carlini wrote:
> Anything you would recommend besides filing a new Bug Report (or marking
> an existing one as regression)?!?

Definitely do that.  I guess it has to do with not recognizing a 
USING_DECL as a template...

Jason