diff mbox

[C++] PR 20420

Message ID 50350248.207@oracle.com
State New
Headers show

Commit Message

Paolo Carlini Aug. 22, 2012, 4:01 p.m. UTC
Hi again,

On 08/22/2012 05:13 PM, Jason Merrill wrote:
> On 08/22/2012 10:55 AM, Paolo Carlini wrote:
>> . thus, in short, what is happening is that, for this testcase:
>>
>> class B
>> {
>> protected:
>>    enum E { E1, E2, E3 };
>> };
>>
>> class D : private B
>> {
>> public:
>>    using B::E;
>>
>> private:
>>    enum E { };
>> };
>>
>> we parse the new declaration enum E { }; and we reach
>> supplement_binding_1 before setting the underlying type of the new
>> declaration. The old declaration is fine, would not ICE 
>> dependent_type_p.
>
> So with your change would we still ICE if D were a template?  It seems 
> like what we should be checking for is null underlying type.
Good question ;) Yesterday night I double checked with this:

template<typename T>
class BT
{
protected:
   enum E { E1, E2, E3 };
   struct S { int i; E e; };
};

template<typename T>
class DT : private BT<T>
{
public:
   using BT<T>::E;
   using BT<T>::S;

private:
   enum E {};
   struct S {};
};

template class DT<int>;

and we handle it correctly (note: we error out *only* at instantiation 
time).

At this point, let me know, I could either add to the testcase a 
templated variant like the above (see attached), or rework the code to 
explicitly check the underlying type (I would add locals hosting the 
TREE_TYPEs to shorten a bit things, etc).

The below passes testing, anyway.

Thanks,
Paolo.

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

Comments

Jason Merrill Aug. 23, 2012, 3:33 a.m. UTC | #1
On 08/22/2012 12:01 PM, Paolo Carlini wrote:
> At this point, let me know, I could either add to the testcase a
> templated variant like the above (see attached), or rework the code to
> explicitly check the underlying type (I would add locals hosting the
> TREE_TYPEs to shorten a bit things, etc).
>
> The below passes testing, anyway.

OK.

Jason
Fabien ChĂȘne Aug. 23, 2012, 8 a.m. UTC | #2
Hi Paolo,

2012/8/23 Jason Merrill <jason@redhat.com>:
[...]
>> The below passes testing, anyway.
>
>
> OK.
>
> Jason

Thanks for tackling this "using" bug !
Incidentally, as you seem to be used to visiting old C++ bugs, feel
free to ping/assign me on bugs that concern using-declarations --
obviously if you do not plan to fix it yourself (which is a better
choice ;-)) and... you are not too hurry to get them fixed ;-)

Thanks again !
Paolo Carlini Aug. 23, 2012, 9:52 a.m. UTC | #3
On 08/23/2012 10:00 AM, Fabien ChĂȘne wrote:
> Thanks for tackling this "using" bug !
You are welcome!
> Incidentally, as you seem to be used to visiting old C++ bugs, feel 
> free to ping/assign me on bugs that concern using-declarations -- 
> obviously if you do not plan to fix it yourself (which is a better 
> choice ;-)) and... you are not too hurry to get them fixed ;-) Thanks 
> again ! 
Often, I'm just looking for simple enough bugs which however - because 
of lack of customer pressure or just plain history - remained opened way 
too much time and we can as well fix once and for all. That said, thanks 
for your message, I'll see what I can do ;) For the time being, I think 
you have already assigned to yourself some pretty serious largish 
issues, thus I will be careful not sending too much your way!

Thanks again!
Paolo.
diff mbox

Patch

Index: testsuite/g++.dg/lookup/using53.C
===================================================================
--- testsuite/g++.dg/lookup/using53.C	(revision 0)
+++ testsuite/g++.dg/lookup/using53.C	(revision 0)
@@ -0,0 +1,53 @@ 
+// PR c++/20420
+
+class B
+{
+protected:
+  enum E { E1, E2, E3 };
+  struct S { int i; E e; };
+};
+
+class D : private B
+{
+public:
+  using B::E;       // { dg-message "previous" }
+  using B::S;       // { dg-message "previous" }
+
+private:
+  enum E {};        // { dg-error "conflicts" }
+  struct S {};      // { dg-error "conflicts" }
+};
+
+template<typename T>
+class BT
+{
+protected:
+  enum E { E1, E2, E3 };
+  struct S { int i; E e; };
+};
+
+template<typename T>
+class DT : private BT<T>
+{
+public:
+  using BT<T>::E;   // { dg-message "previous" }
+  using BT<T>::S;   // { dg-message "previous" }
+
+private:
+  enum E {};        // { dg-error "conflicts" }
+  struct S {};      // { dg-error "conflicts" }
+};
+
+template class DT<int>;
+
+namespace N
+{
+  int i;
+}
+
+void
+f ()
+{
+  using N::i;
+  using N::i;       // { dg-error "declared" }
+}
Index: cp/name-lookup.c
===================================================================
--- cp/name-lookup.c	(revision 190590)
+++ cp/name-lookup.c	(working copy)
@@ -441,7 +441,8 @@  supplement_binding_1 (cxx_binding *binding, tree d
 	     template in order to handle late matching of underlying
 	     type on an opaque-enum-declaration followed by an
 	     enum-specifier.  */
-	  || (TREE_CODE (TREE_TYPE (target_decl)) == ENUMERAL_TYPE
+	  || (processing_template_decl
+	      && TREE_CODE (TREE_TYPE (target_decl)) == ENUMERAL_TYPE
 	      && TREE_CODE (TREE_TYPE (target_bval)) == ENUMERAL_TYPE
 	      && (dependent_type_p (ENUM_UNDERLYING_TYPE
 				    (TREE_TYPE (target_decl)))
@@ -2420,7 +2421,15 @@  validate_nonmember_using_decl (tree decl, tree sco
   gcc_assert (DECL_P (decl));
 
   /* Make a USING_DECL.  */
-  return push_using_decl (scope, name);
+  tree using_decl = push_using_decl (scope, name);
+
+  if (using_decl == NULL_TREE
+      && at_function_scope_p ()
+      && TREE_CODE (decl) == VAR_DECL)
+    /* C++11 7.3.3/10.  */
+    error ("%qD is already declared in this scope", name);
+  
+  return using_decl;
 }
 
 /* Process local and global using-declarations.  */