r199490 - Permit redeclaration of tags introduced by using decls

[Alp Toker]

I think we're OK here. 3.3.1/4 describes the rule as applying to a set 
of declarations "each of which specifies the same unqualified name" and 
we're guaranteed that using declarations will be qualified while the 
redeclarations will be unqualified (and the shadows are an 
implementation detail). So there's no conflict even in a pedantic reading..

The "namespace surrounding the declaration" rule also doesn't appear to 
apply here given that one of the purposes of using declarations to work 
around strict lexical scope. Perhaps shades of PR17337 here but I'm 
tending to side with the mainstream again.

Like you say the the standard is unclear but with this reading we get 
compatibility with g++ and MSVC and it does "feel right".

Awaiting that clarification you requested from the core reflector -- if 
anything it's an interesting discussion.

Alp.


On 17/01/2014 20:59, Richard Smith wrote:
> Hi Alp,
>
> This change doesn't look correct.
>
> On Fri Jan 17 2014 at 5:04:31 AM, Alp Toker <alp at nuanti.com 
> <mailto:alp at nuanti.com>> wrote:
>
>     Author: alp
>     Date: Fri Jan 17 06:57:21 2014
>     New Revision: 199490
>
>     URL: http://llvm.org/viewvc/llvm-project?rev=199490&view=rev
>     Log:
>     Permit redeclaration of tags introduced by using decls
>
>     This valid construct appears in MSVC headers where it's used to
>     provide a
>     definition for the '::type_info' compiler builtin type.
>
>     Modified:
>         cfe/trunk/lib/Sema/SemaDecl.cpp
>         cfe/trunk/test/SemaCXX/using-decl-1.cpp
>
>     Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>     URL:
>     http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=199490&r1=199489&r2=199490&view=diff
>     ==============================================================================
>     --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>     +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Jan 17 06:57:21 2014
>     @@ -10681,7 +10681,8 @@ Decl *Sema::ActOnTag(Scope *S, unsigned
>        }
>
>        if (!Previous.empty()) {
>     -    NamedDecl *PrevDecl = (*Previous.begin())->getUnderlyingDecl();
>     +    NamedDecl *DirectPrevDecl = *Previous.begin();
>     +    NamedDecl *PrevDecl = DirectPrevDecl->getUnderlyingDecl();
>
>          // It's okay to have a tag decl in the same scope as a typedef
>          // which hides a tag decl in the same scope.  Finding this
>     @@ -10713,7 +10714,7 @@ Decl *Sema::ActOnTag(Scope *S, unsigned
>            // in the same scope (so that the definition/declaration
>     completes or
>            // rementions the tag), reuse the decl.
>            if (TUK == TUK_Reference || TUK == TUK_Friend ||
>     -          isDeclInScope(PrevDecl, SearchDC, S,
>     +          isDeclInScope(DirectPrevDecl, SearchDC, S,
>                              SS.isNotEmpty() ||
>     isExplicitSpecialization)) {
>              // Make sure that this wasn't declared as an enum and now
>     used as a
>              // struct or something similar.
>
>     Modified: cfe/trunk/test/SemaCXX/using-decl-1.cpp
>     URL:
>     http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/using-decl-1.cpp?rev=199490&r1=199489&r2=199490&view=diff
>     ==============================================================================
>     --- cfe/trunk/test/SemaCXX/using-decl-1.cpp (original)
>     +++ cfe/trunk/test/SemaCXX/using-decl-1.cpp Fri Jan 17 06:57:21 2014
>     @@ -119,6 +119,27 @@ namespace foo
>        };
>      }
>
>     +namespace using_tag_redeclaration
>     +{
>     +  struct S;
>     +  namespace N {
>     +    using ::using_tag_redeclaration::S;
>     +    struct S {}; // expected-note {{previous definition is here}}
>
>
> This is ill-formed, by 3.3.1/4. It appears that your change makes the 
> second declaration into a redeclaration of 
> ::using_tag_redeclaration::S; that's incorrect. (Our previous handling 
> of this case was also incorrect, albeit in a different way, since we 
> didn't enforce the 3.3.1/4 rule here.)
>
> If we need to handle something of this form for MSVC compatibility, 
> we'll need to put it behind MSVCCompat.
>
>     +  }
>     +  void f() {
>     +    N::S s1;
>
>     +    S s2;
>     +  }
>     +  void g() {
>     +    struct S; // expected-note {{forward declaration of 'S'}}
>     +    S s3; // expected-error {{variable has incomplete type 'S'}}
>     +  }
>     +  void h() {
>     +    using ::using_tag_redeclaration::S;
>     +    struct S {}; // expected-error {{redefinition of 'S'}}
>
>     +  }
>     +}
>     +
>      // Don't suggest non-typenames for positions requiring typenames.
>      namespace using_suggestion_tyname_val {
>      namespace N { void FFF() {} }
>
>
>     _______________________________________________
>     cfe-commits mailing list
>     cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>     http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>

-- 
http://www.nuanti.com
the browser experts