r199490 - Permit redeclaration of tags introduced by using decls

Alp Toker alp at nuanti.com
Fri Jan 17 17:09:55 PST 2014


On 18/01/2014 00:56, Richard Smith wrote:
> On Fri Jan 17 2014 at 4:41:22 PM, Alp Toker <alp at nuanti.com 
> <mailto:alp at nuanti.com>> wrote:
>
>     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..
>
>
> 7.3.3/13, and its example, clarify that using-declarations are 
> considered here. I've mailed the core reflector to get the wording in 
> 3.3.1/4 clarified.
>
>     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".
>
>
> EDG, g++, and MSVC all have different behavior here (different from 
> each other and different from us).
>
>     Awaiting that clarification you requested from the core reflector
>     -- if
>     anything it's an interesting discussion.
>
>
> Discussion on core so far suggests that an unqualified name *always* 
> declares a name in the lexical context in which it appears, meaning 
> our former behavior on this example was more appropriate than our new 
> behavior.

Thanks for the indication, I've restricted the new behaviour to MSVCMode 
in r199531 for now.

Will shortly also post the equivalent fix for PR13786.

Alp.


>
>     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>
>     > <mailto: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>
>     <mailto: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
>

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




More information about the cfe-commits mailing list