[cfe-commits] r58916 - in /cfe/trunk: include/clang/AST/DeclBase.h include/clang/AST/DeclCXX.h include/clang/Basic/DiagnosticKinds.def lib/Sema/Sema.cpp lib/Sema/Sema.h lib/Sema/SemaCXXScopeSpec.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExpr.cpp test/SemaCXX/nested-name-spec.cpp

[Douglas Gregor]

On Nov 8, 2008, at 12:17 PM, Argiris Kirtzidis wrote:

> Author: akirtzidis
> Date: Sat Nov  8 11:17:31 2008
> New Revision: 58916
>
> URL: http://llvm.org/viewvc/llvm-project?rev=58916&view=rev
> Log:
> Implement Sema support for C++ nested-name-specifiers.

Very cool.

> +  bool isFileContext() const {
> +    return DeclKind == Decl::TranslationUnit || DeclKind ==  
> Decl::Namespace;
> +  }

isFileContext seems like a weird name for this function... I would  
have called it isNamespaceContext, since the translation unit context  
is the same as the global namespace context for name lookup's purposes.

> +DIAG(err_not_tag_in_scope, ERROR,
> +     "'%0' does not name a tag member in the specified scope")

The term "tag" is very much a compiler-writers term. Could we replace  
tag with '%1' and fill in the appropriate tag kind (struct, class,  
union, enum)?

> +  /// PreDeclaratorDC - Keeps the declaration context before  
> switching to the
> +  /// context of a declarator's nested-name-specifier.
> +  DeclContext *PreDeclaratorDC;

I assume that we'll eventually need this to be a stack of DeclContext  
*'s, to deal with something like:

	::foo::bar<wibble::wonka>::type

Perhaps we should just make it a stack now so we're ready for the  
future.

> +namespace {
> +  Decl *LookupNestedName(DeclContext *LookupCtx, bool  
> LookInParentCtx,
> +                         const IdentifierInfo &II, bool  
> &IdIsUndeclared) {
>
> +    IdentifierResolver::iterator
> +      I = IdentifierResolver::begin(&II, LookupCtx, LookInParentCtx),
> +      E = IdentifierResolver::end();
> +
> +    if (I == E) {
> +      IdIsUndeclared = true;
> +      return 0;
> +    }
> +    IdIsUndeclared = false;
> +
> +    // C++ 3.4.3p1 :
> +    // During the lookup for a name preceding the :: scope  
> resolution operator,
> +    // object, function, and enumerator names are ignored. If the  
> name found is
> +    // not a class-name or namespace-name, the program is ill-formed.
> +
> +    for (; I != E; ++I) {
> +      if (TypedefDecl *TD = dyn_cast<TypedefDecl>(*I)) {
> +        if (TD->getUnderlyingType()->isRecordType())
> +          break;
> +        continue;
> +      }
> +      if (((*I)->getIdentifierNamespace()&Decl::IDNS_Tag) && ! 
> isa<EnumDecl>(*I))
> +        break;
> +    }

I don't think this is quite right for C++03. The standard says that we  
ignore enumerator names (e.g., an EnumConstantDecl), but the check in  
the "if" is skipping enumeration names (EnumDecl). Here's an example  
where it matters:

namespace N {
   struct C {
     static float X;
   };

   typedef C Typedef;

   namespace M {
     enum E {
       X = 17
     };

     typedef E Typedef;

     void test() {
       float& f = Typedef::X;
     }
   }
}

GCC rejects this example:

/Users/dgregor/foo.C: In function ‘void N::M::test()’:
/Users/dgregor/foo.C:16: error: ‘Typedef’ is not a class or namespace

Interestingly, EDG accepts the example, and so does Clang, but I'm  
pretty certain that both are wrong. Qualified name lookup should find  
the typedef for the enumeration 'E', and then fail because we can't do  
qualified name lookup into an enumeration type in C++03. In C++0x, we  
*can* do qualified name lookup into an enumeration, so the example  
fails because we can't bind an 'int' to a reference to a 'float'.

For Clang, I think the fix is to remove that "isa" check and add a "||  
TD->getUnderlyingType()->isEnumeralType()" into the "if" inside the  
Typedef-checking block, then cope with the difference between C++03  
and C++0x semantics in the caller.

> +  // See if this is a redefinition of a variable in the same scope.
> +  if (!D.getCXXScopeSpec().isSet()) {
> +    DC = CurContext;
> +    PrevDecl = LookupDecl(II, Decl::IDNS_Ordinary, S);
> +  } else { // Something like "int foo::x;"
> +    DC =  
> static_cast<DeclContext*>(D.getCXXScopeSpec().getScopeRep());
> +    PrevDecl = LookupDecl(II, Decl::IDNS_Ordinary, S, DC);
> +
> +    // C++ 7.3.1.2p2:
> +    // Members (including explicit specializations of templates) of  
> a named
> +    // namespace can also be defined outside that namespace by  
> explicit
> +    // qualification of the name being defined, provided that the  
> entity being
> +    // defined was already declared in the namespace and the  
> definition appears
> +    // after the point of declaration in a namespace that encloses  
> the
> +    // declarations namespace.
> +    //
> +    if (PrevDecl == 0) {
> +      // No previous declaration in the qualifying scope.
> +      Diag(D.getIdentifierLoc(), diag::err_typecheck_no_member,
> +           II->getName(), D.getCXXScopeSpec().getRange());
> +    } else if (!CurContext->Encloses(DC)) {
> +      // The qualifying scope doesn't enclose the original  
> declaration.
> +      // Emit diagnostic based on current scope.
> +      SourceLocation L = D.getIdentifierLoc();
> +      SourceRange R = D.getCXXScopeSpec().getRange();
> +      if (isa<FunctionDecl>(CurContext)) {
> +        Diag(L, diag::err_invalid_declarator_in_function, II- 
> >getName(), R);
> +      } else {
> +      Diag(L, diag::err_invalid_declarator_scope, II->getName(),
> +           cast<NamedDecl>(DC)->getName(), R);
> +      }
> +    }
> +  }

Hey, nice! I didn't expect to see this in here :)


	- Doug