[cfe-commits] r102164 - in /cfe/trunk: include/clang/AST/DeclBase.h include/clang/Basic/DiagnosticSemaKinds.td lib/AST/ASTImporter.cpp lib/AST/DeclBase.cpp lib/Sema/SemaCodeComplete.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaLookup.cpp test/CXX/basic/basic.lookup/basic.lookup.elab/p2.cpp test/CXX/class.access/class.friend/p1.cpp test/CXX/temp/temp.decls/temp.friend/p1.cpp test/SemaCXX/typedef-redecl.cpp
Douglas Gregor
dgregor at apple.com
Fri Apr 23 08:52:10 PDT 2010
On Apr 22, 2010, at 7:41 PM, John McCall wrote:
> Author: rjmccall
> Date: Thu Apr 22 21:41:41 2010
> New Revision: 102164
>
> URL: http://llvm.org/viewvc/llvm-project?rev=102164&view=rev
> Log:
> C++ doesn't really use "namespaces" for different kinds of names the same
> way that C does. Among other differences, elaborated type specifiers
> are defined to skip "non-types", which, as you might imagine, does not
> include typedefs. Rework our use of IDNS masks to capture the semantics
> of different kinds of declarations better, and remove most current lookup
> filters. Removing the last remaining filter is more complicated and will
> happen in a separate patch.
>
> Fixes PR 6885 as well some spectrum of unfiled bugs.
Very cool. One comment below about the bootstrap failures.
>
> Added:
> cfe/trunk/test/CXX/basic/basic.lookup/basic.lookup.elab/p2.cpp
> Modified:
> cfe/trunk/include/clang/AST/DeclBase.h
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/AST/ASTImporter.cpp
> cfe/trunk/lib/AST/DeclBase.cpp
> cfe/trunk/lib/Sema/SemaCodeComplete.cpp
> cfe/trunk/lib/Sema/SemaDecl.cpp
> cfe/trunk/lib/Sema/SemaLookup.cpp
> cfe/trunk/test/CXX/class.access/class.friend/p1.cpp
> cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p1.cpp
> cfe/trunk/test/SemaCXX/typedef-redecl.cpp
>
> Modified: cfe/trunk/include/clang/AST/DeclBase.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=102164&r1=102163&r2=102164&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/DeclBase.h (original)
> +++ cfe/trunk/include/clang/AST/DeclBase.h Thu Apr 22 21:41:41 2010
> @@ -76,24 +76,63 @@
> #include "clang/AST/DeclNodes.def"
> };
>
> - /// IdentifierNamespace - According to C99 6.2.3, there are four
> - /// namespaces, labels, tags, members and ordinary
> - /// identifiers. These are meant as bitmasks, so that searches in
> - /// C++ can look into the "tag" namespace during ordinary lookup. We
> - /// use additional namespaces for Objective-C entities. We also put
> - /// C++ friend declarations (of previously-undeclared entities) in
> - /// shadow namespaces, and 'using' declarations (as opposed to their
> - /// implicit shadow declarations) can be found in their own
> - /// namespace.
> + /// IdentifierNamespace - The different namespaces in which
> + /// declarations may appear. According to C99 6.2.3, there are
> + /// four namespaces, labels, tags, members and ordinary
> + /// identifiers. C++ describes lookup completely differently:
> + /// certain lookups merely "ignore" certain kinds of declarations,
> + /// usually based on whether the declaration is of a type, etc.
> + ///
> + /// These are meant as bitmasks, so that searches in
> + /// C++ can look into the "tag" namespace during ordinary lookup.
> + ///
> + /// Decl currently provides 16 bits of IDNS bits.
> enum IdentifierNamespace {
> - IDNS_Label = 0x1,
> - IDNS_Tag = 0x2,
> - IDNS_Member = 0x4,
> - IDNS_Ordinary = 0x8,
> - IDNS_ObjCProtocol = 0x10,
> - IDNS_OrdinaryFriend = 0x80,
> - IDNS_TagFriend = 0x100,
> - IDNS_Using = 0x200
> + /// Labels, declared with 'x:' and referenced with 'goto x'.
> + IDNS_Label = 0x0001,
> +
> + /// Tags, declared with 'struct foo;' and referenced with
> + /// 'struct foo'. All tags are also types. This is what
> + /// elaborated-type-specifiers look for in C.
> + IDNS_Tag = 0x0002,
> +
> + /// Types, declared with 'struct foo', typedefs, etc.
> + /// This is what elaborated-type-specifiers look for in C++,
> + /// but note that it's ill-formed to find a non-tag.
> + IDNS_Type = 0x0004,
> +
> + /// Members, declared with object declarations within tag
> + /// definitions. In C, these can only be found by "qualified"
> + /// lookup in member expressions. In C++, they're found by
> + /// normal lookup.
> + IDNS_Member = 0x0008,
> +
> + /// Namespaces, declared with 'namespace foo {}'.
> + /// Lookup for nested-name-specifiers find these.
> + IDNS_Namespace = 0x0010,
> +
> + /// Ordinary names. In C, everything that's not a label, tag,
> + /// or member ends up here.
> + IDNS_Ordinary = 0x0020,
> +
> + /// Objective C @protocol.
> + IDNS_ObjCProtocol = 0x0040,
> +
> + /// This declaration is a friend function. A friend function
> + /// declaration is always in this namespace but may also be in
> + /// IDNS_Ordinary if it was previously declared.
> + IDNS_OrdinaryFriend = 0x0080,
> +
> + /// This declaration is a friend class. A friend class
> + /// declaration is always in this namespace but may also be in
> + /// IDNS_Tag|IDNS_Type if it was previously declared.
> + IDNS_TagFriend = 0x0100,
> +
> + /// This declaration is a using declaration. A using declaration
> + /// *introduces* a number of other declarations into the current
> + /// scope, and those declarations use the IDNS of their targets,
> + /// but the actual using declarations go in this namespace.
> + IDNS_Using = 0x0200
> };
>
> /// ObjCDeclQualifier - Qualifier used on types in method declarations
> @@ -453,14 +492,14 @@
> assert((OldNS & (IDNS_Tag | IDNS_Ordinary |
> IDNS_TagFriend | IDNS_OrdinaryFriend)) &&
> "namespace includes neither ordinary nor tag");
> - assert(!(OldNS & ~(IDNS_Tag | IDNS_Ordinary |
> + assert(!(OldNS & ~(IDNS_Tag | IDNS_Ordinary | IDNS_Type |
> IDNS_TagFriend | IDNS_OrdinaryFriend)) &&
> "namespace includes other than ordinary or tag");
>
> IdentifierNamespace = 0;
> if (OldNS & (IDNS_Tag | IDNS_TagFriend)) {
> IdentifierNamespace |= IDNS_TagFriend;
> - if (PreviouslyDeclared) IdentifierNamespace |= IDNS_Tag;
> + if (PreviouslyDeclared) IdentifierNamespace |= IDNS_Tag | IDNS_Type;
> }
>
> if (OldNS & (IDNS_Ordinary | IDNS_OrdinaryFriend)) {
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=102164&r1=102163&r2=102164&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Apr 22 21:41:41 2010
> @@ -1583,6 +1583,11 @@
> "redefinition of %0 as different kind of symbol">;
> def err_redefinition_different_typedef : Error<
> "typedef redefinition with different types (%0 vs %1)">;
> +def err_tag_reference_non_tag : Error<
> + "elaborated type refers to %select{a non-tag type|a typedef|a template}0">;
> +def err_tag_reference_conflict : Error<
> + "implicit declaration introduced by elaborated type conflicts with "
> + "%select{a declaration|a typedef|a template}0 of the same name">;
> def err_tag_definition_of_typedef : Error<
> "definition of type %0 conflicts with typedef of the same name">;
> def err_conflicting_types : Error<"conflicting types for %0">;
>
> Modified: cfe/trunk/lib/AST/ASTImporter.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=102164&r1=102163&r2=102164&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/ASTImporter.cpp (original)
> +++ cfe/trunk/lib/AST/ASTImporter.cpp Thu Apr 22 21:41:41 2010
> @@ -1438,7 +1438,7 @@
> for (DeclContext::lookup_result Lookup = DC->lookup(Name);
> Lookup.first != Lookup.second;
> ++Lookup.first) {
> - if (!(*Lookup.first)->isInIdentifierNamespace(Decl::IDNS_Ordinary))
> + if (!(*Lookup.first)->isInIdentifierNamespace(Decl::IDNS_Namespace))
> continue;
>
> if (NamespaceDecl *FoundNS = dyn_cast<NamespaceDecl>(*Lookup.first)) {
> @@ -1451,7 +1451,7 @@
> }
>
> if (!ConflictingDecls.empty()) {
> - Name = Importer.HandleNameConflict(Name, DC, Decl::IDNS_Ordinary,
> + Name = Importer.HandleNameConflict(Name, DC, Decl::IDNS_Namespace,
> ConflictingDecls.data(),
> ConflictingDecls.size());
> }
>
> Modified: cfe/trunk/lib/AST/DeclBase.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=102164&r1=102163&r2=102164&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/DeclBase.cpp (original)
> +++ cfe/trunk/lib/AST/DeclBase.cpp Thu Apr 22 21:41:41 2010
> @@ -231,23 +231,28 @@
> case CXXConstructor:
> case CXXDestructor:
> case CXXConversion:
> - case Typedef:
> case EnumConstant:
> case Var:
> case ImplicitParam:
> case ParmVar:
> case NonTypeTemplateParm:
> case ObjCMethod:
> - case ObjCInterface:
> case ObjCProperty:
> - case ObjCCompatibleAlias:
> return IDNS_Ordinary;
>
> + case ObjCCompatibleAlias:
> + case ObjCInterface:
> + return IDNS_Ordinary | IDNS_Type;
> +
> + case Typedef:
> + case UnresolvedUsingTypename:
> + case TemplateTypeParm:
> + return IDNS_Ordinary | IDNS_Type;
> +
> case UsingShadow:
> return 0; // we'll actually overwrite this later
>
> case UnresolvedUsingValue:
> - case UnresolvedUsingTypename:
> return IDNS_Ordinary | IDNS_Using;
>
> case Using:
> @@ -264,15 +269,18 @@
> case Record:
> case CXXRecord:
> case Enum:
> - case TemplateTypeParm:
> - return IDNS_Tag;
> + return IDNS_Tag | IDNS_Type;
>
> case Namespace:
> + case NamespaceAlias:
> + return IDNS_Namespace;
> +
> case FunctionTemplate:
> + return IDNS_Ordinary;
> +
> case ClassTemplate:
> case TemplateTemplateParm:
> - case NamespaceAlias:
> - return IDNS_Tag | IDNS_Ordinary;
> + return IDNS_Ordinary | IDNS_Tag | IDNS_Type;
>
> // Never have names.
> case Friend:
>
> Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=102164&r1=102163&r2=102164&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Thu Apr 22 21:41:41 2010
> @@ -415,6 +415,9 @@
>
> return false;
> }
> +
> + if (Filter == &ResultBuilder::IsNestedNameSpecifier)
> + AsNestedNameSpecifier = true;
>
> // ... then it must be interesting!
> return true;
> @@ -504,7 +507,8 @@
> }
> for (; I != IEnd; ++I) {
> // A tag declaration does not hide a non-tag declaration.
> - if (I->first->getIdentifierNamespace() == Decl::IDNS_Tag &&
> + if (I->first->getIdentifierNamespace()
> + == (Decl::IDNS_Tag | Decl::IDNS_Type) &&
> (IDNS & (Decl::IDNS_Member | Decl::IDNS_Ordinary |
> Decl::IDNS_ObjCProtocol)))
> continue;
> @@ -629,7 +633,7 @@
> bool ResultBuilder::IsOrdinaryName(NamedDecl *ND) const {
> unsigned IDNS = Decl::IDNS_Ordinary;
> if (SemaRef.getLangOptions().CPlusPlus)
> - IDNS |= Decl::IDNS_Tag;
> + IDNS |= Decl::IDNS_Tag | Decl::IDNS_Namespace;
> else if (SemaRef.getLangOptions().ObjC1 && isa<ObjCIvarDecl>(ND))
> return true;
>
> @@ -641,7 +645,7 @@
> bool ResultBuilder::IsOrdinaryNonValueName(NamedDecl *ND) const {
> unsigned IDNS = Decl::IDNS_Ordinary;
> if (SemaRef.getLangOptions().CPlusPlus)
> - IDNS |= Decl::IDNS_Tag;
> + IDNS |= Decl::IDNS_Tag | Decl::IDNS_Namespace;
>
> return (ND->getIdentifierNamespace() & IDNS) &&
> !isa<ValueDecl>(ND) && !isa<FunctionTemplateDecl>(ND);
> @@ -2094,10 +2098,16 @@
> return;
> }
>
> - ResultBuilder Results(*this, Filter);
> - Results.allowNestedNameSpecifiers();
> + ResultBuilder Results(*this);
> CodeCompletionDeclConsumer Consumer(Results, CurContext);
> +
> + // First pass: look for tags.
> + Results.setFilter(Filter);
> LookupVisibleDecls(S, LookupTagName, Consumer);
> +
> + // Second pass: look for nested name specifiers.
> + Results.setFilter(&ResultBuilder::IsNestedNameSpecifier);
> + LookupVisibleDecls(S, LookupNestedNameSpecifierName, Consumer);
>
> HandleCodeCompleteResults(this, CodeCompleter, Results.data(),Results.size());
> }
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=102164&r1=102163&r2=102164&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Apr 22 21:41:41 2010
> @@ -5000,7 +5000,8 @@
> SearchDC = SearchDC->getEnclosingNamespaceContext();
> }
>
> - // In C++, look for a shadow friend decl.
> + // In C++, we need to do a redeclaration lookup to properly
> + // diagnose some problems.
> if (getLangOptions().CPlusPlus) {
> Previous.setRedeclarationKind(ForRedeclaration);
> LookupQualifiedName(Previous, SearchDC);
> @@ -5009,6 +5010,26 @@
>
> if (!Previous.empty()) {
> NamedDecl *PrevDecl = (*Previous.begin())->getUnderlyingDecl();
> +
> + // It's okay to have a tag decl in the same scope as a typedef
> + // which shadows a tag decl in the same scope. Finding this
> + // insanity with a redeclaration lookup can only actually happen
> + // in C++.
> + if (getLangOptions().CPlusPlus &&
> + TUK != TUK_Reference && TUK != TUK_Friend) {
> + if (TypedefDecl *TD = dyn_cast<TypedefDecl>(PrevDecl)) {
> + if (const TagType *TT = TD->getUnderlyingType()->getAs<TagType>()) {
> + TagDecl *Tag = TT->getDecl();
> + if (Tag->getDeclName() == Name &&
> + Tag->getDeclContext()->Equals(SearchDC)) {
> + PrevDecl = Tag;
> + Previous.clear();
> + Previous.addDecl(Tag);
> + }
> + }
> + }
> + }
This is probably the cause of
http://llvm.org/bugs/show_bug.cgi?id=6901
My intuition says that the TUK != TUK_Reference check shouldn't be there, because one should be able to refer to a tag typedef'd to itself via an elaborated-type-specifier. That goes against 9.1p5 and 7.1.6.3p2, but it's in line with existing practice, e.g., GCC and EDG accept:
typedef struct X { } X;
struct X *x1;
X *x2;
struct X;
but reject
struct X;
namespace N {
typedef struct X X;
struct X *x1;
X *x2;
struct X;
}
So it seems like the "typedef a tag type to itself" of 7.1.3 [dcl.typedef]p3 and p4 somehow imbue the typedef-name with magical class-name powers that aren't explained anywhere. Seems like a defect to me, since reading 9.1p5 and 7.1.6.3p2 literally is a significant departure from C semantics and from existing practice.
There is an open issue about exactly this problem, here:
http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#407
The committee's direction seems to be that we should permit the use of a typedef-name in an elaborated-type-specifier only if it is declared in the same scope as the class or enumeration it names.
- Doug
More information about the cfe-commits
mailing list