On 18/01/2014 00:56, Richard Smith wrote:
On Fri Jan 17 2014 at 4:41:22 PM, Alp Toker <[email protected] <mailto:[email protected]>> 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 <[email protected]
    <mailto:[email protected]>
    > <mailto:[email protected] <mailto:[email protected]>>> 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
    > [email protected] <mailto:[email protected]>
    <mailto:[email protected] <mailto:[email protected]>>
    > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
    >

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


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

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to