r228661 - [modules] When determining whether a name from a module replaces a name we
Richard Smith
richard-llvm at metafoo.co.uk
Mon Feb 9 19:28:11 PST 2015
Author: rsmith
Date: Mon Feb 9 21:28:10 2015
New Revision: 228661
URL: http://llvm.org/viewvc/llvm-project?rev=228661&view=rev
Log:
[modules] When determining whether a name from a module replaces a name we
already have, check whether the name from the module is actually newer than the
existing declaration. If it isn't, we might (say) replace a visible declaration
with an injected friend, and thus make it invisible (or lose a default argument
or an array bound).
Added:
cfe/trunk/test/Modules/Inputs/cxx-lookup/na.h
cfe/trunk/test/Modules/Inputs/cxx-lookup/nb.h
Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/DeclContextInternals.h
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/AST/DeclBase.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
cfe/trunk/test/Modules/Inputs/cxx-lookup/module.modulemap
cfe/trunk/test/Modules/cxx-lookup.cpp
Modified: cfe/trunk/include/clang/AST/Decl.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=228661&r1=228660&r2=228661&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Mon Feb 9 21:28:10 2015
@@ -179,14 +179,17 @@ public:
const PrintingPolicy &Policy,
bool Qualified) const;
- /// declarationReplaces - Determine whether this declaration, if
+ /// \brief Determine whether this declaration, if
/// known to be well-formed within its context, will replace the
/// declaration OldD if introduced into scope. A declaration will
/// replace another declaration if, for example, it is a
/// redeclaration of the same variable or function, but not if it is
/// a declaration of a different kind (function vs. class) or an
/// overloaded function.
- bool declarationReplaces(NamedDecl *OldD) const;
+ ///
+ /// \param IsKnownNewer \c true if this declaration is known to be newer
+ /// than \p OldD (for instance, if this declaration is newly-created).
+ bool declarationReplaces(NamedDecl *OldD, bool IsKnownNewer = true) const;
/// \brief Determine whether this declaration has linkage.
bool hasLinkage() const;
Modified: cfe/trunk/include/clang/AST/DeclContextInternals.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclContextInternals.h?rev=228661&r1=228660&r2=228661&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclContextInternals.h (original)
+++ cfe/trunk/include/clang/AST/DeclContextInternals.h Mon Feb 9 21:28:10 2015
@@ -163,10 +163,10 @@ public:
/// HandleRedeclaration - If this is a redeclaration of an existing decl,
/// replace the old one with D and return true. Otherwise return false.
- bool HandleRedeclaration(NamedDecl *D) {
+ bool HandleRedeclaration(NamedDecl *D, bool IsKnownNewer) {
// Most decls only have one entry in their list, special case it.
if (NamedDecl *OldD = getAsDecl()) {
- if (!D->declarationReplaces(OldD))
+ if (!D->declarationReplaces(OldD, IsKnownNewer))
return false;
setOnlyValue(D);
return true;
@@ -177,7 +177,7 @@ public:
for (DeclsTy::iterator OD = Vec.begin(), ODEnd = Vec.end();
OD != ODEnd; ++OD) {
NamedDecl *OldD = *OD;
- if (D->declarationReplaces(OldD)) {
+ if (D->declarationReplaces(OldD, IsKnownNewer)) {
*OD = D;
return true;
}
Modified: cfe/trunk/lib/AST/Decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=228661&r1=228660&r2=228661&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Mon Feb 9 21:28:10 2015
@@ -1445,74 +1445,122 @@ void NamedDecl::getNameForDiagnostic(raw
printName(OS);
}
-bool NamedDecl::declarationReplaces(NamedDecl *OldD) const {
- assert(getDeclName() == OldD->getDeclName() && "Declaration name mismatch");
+static bool isKindReplaceableBy(Decl::Kind OldK, Decl::Kind NewK) {
+ // For method declarations, we never replace.
+ if (ObjCMethodDecl::classofKind(NewK))
+ return false;
- // UsingDirectiveDecl's are not really NamedDecl's, and all have same name.
- // We want to keep it, unless it nominates same namespace.
- if (getKind() == Decl::UsingDirective) {
- return cast<UsingDirectiveDecl>(this)->getNominatedNamespace()
- ->getOriginalNamespace() ==
- cast<UsingDirectiveDecl>(OldD)->getNominatedNamespace()
- ->getOriginalNamespace();
+ if (OldK == NewK)
+ return true;
+
+ // A compatibility alias for a class can be replaced by an interface.
+ if (ObjCCompatibleAliasDecl::classofKind(OldK) &&
+ ObjCInterfaceDecl::classofKind(NewK))
+ return true;
+
+ // A typedef-declaration, alias-declaration, or Objective-C class declaration
+ // can replace another declaration of the same type. Semantic analysis checks
+ // that we have matching types.
+ if ((TypedefNameDecl::classofKind(OldK) ||
+ ObjCInterfaceDecl::classofKind(OldK)) &&
+ (TypedefNameDecl::classofKind(NewK) ||
+ ObjCInterfaceDecl::classofKind(NewK)))
+ return true;
+
+ // Otherwise, a kind mismatch implies that the declaration is not replaced.
+ return false;
+}
+
+template<typename T> static bool isRedeclarableImpl(Redeclarable<T> *) {
+ return true;
+}
+static bool isRedeclarableImpl(...) { return false; }
+static bool isRedeclarable(Decl::Kind K) {
+ switch (K) {
+#define DECL(Type, Base) \
+ case Decl::Type: \
+ return isRedeclarableImpl((Type##Decl *)nullptr);
+#define ABSTRACT_DECL(DECL)
+#include "clang/AST/DeclNodes.inc"
}
+ llvm_unreachable("unknown decl kind");
+}
- if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(this))
- // For function declarations, we keep track of redeclarations.
- return FD->getPreviousDecl() == OldD;
+bool NamedDecl::declarationReplaces(NamedDecl *OldD, bool IsKnownNewer) const {
+ assert(getDeclName() == OldD->getDeclName() && "Declaration name mismatch");
- // For function templates, the underlying function declarations are linked.
- if (const FunctionTemplateDecl *FunctionTemplate
- = dyn_cast<FunctionTemplateDecl>(this))
- if (const FunctionTemplateDecl *OldFunctionTemplate
- = dyn_cast<FunctionTemplateDecl>(OldD))
- return FunctionTemplate->getTemplatedDecl()
- ->declarationReplaces(OldFunctionTemplate->getTemplatedDecl());
+ if (!isKindReplaceableBy(OldD->getKind(), getKind()))
+ return false;
- // For method declarations, we keep track of redeclarations.
- if (isa<ObjCMethodDecl>(this))
+ // Inline namespaces can give us two declarations with the same
+ // name and kind in the same scope but different contexts; we should
+ // keep both declarations in this case.
+ if (!this->getDeclContext()->getRedeclContext()->Equals(
+ OldD->getDeclContext()->getRedeclContext()))
return false;
- // FIXME: Is this correct if one of the decls comes from an inline namespace?
- if (isa<ObjCInterfaceDecl>(this) && isa<ObjCCompatibleAliasDecl>(OldD))
- return true;
+ if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(this))
+ // For function declarations, we keep track of redeclarations.
+ // FIXME: This returns false for functions that should in fact be replaced.
+ // Instead, perform some kind of type check?
+ if (FD->getPreviousDecl() != OldD)
+ return false;
- if (isa<UsingShadowDecl>(this) && isa<UsingShadowDecl>(OldD))
- return cast<UsingShadowDecl>(this)->getTargetDecl() ==
- cast<UsingShadowDecl>(OldD)->getTargetDecl();
+ // For function templates, the underlying function declarations are linked.
+ if (const FunctionTemplateDecl *FunctionTemplate =
+ dyn_cast<FunctionTemplateDecl>(this))
+ return FunctionTemplate->getTemplatedDecl()->declarationReplaces(
+ cast<FunctionTemplateDecl>(OldD)->getTemplatedDecl());
+
+ // Using shadow declarations can be overloaded on their target declarations
+ // if they introduce functions.
+ // FIXME: If our target replaces the old target, can we replace the old
+ // shadow declaration?
+ if (auto *USD = dyn_cast<UsingShadowDecl>(this))
+ if (USD->getTargetDecl() != cast<UsingShadowDecl>(OldD)->getTargetDecl())
+ return false;
- if (isa<UsingDecl>(this) && isa<UsingDecl>(OldD)) {
+ // Using declarations can be overloaded if they introduce functions.
+ if (auto *UD = dyn_cast<UsingDecl>(this)) {
ASTContext &Context = getASTContext();
- return Context.getCanonicalNestedNameSpecifier(
- cast<UsingDecl>(this)->getQualifier()) ==
+ return Context.getCanonicalNestedNameSpecifier(UD->getQualifier()) ==
Context.getCanonicalNestedNameSpecifier(
- cast<UsingDecl>(OldD)->getQualifier());
+ cast<UsingDecl>(OldD)->getQualifier());
}
-
- if (isa<UnresolvedUsingValueDecl>(this) &&
- isa<UnresolvedUsingValueDecl>(OldD)) {
+ if (auto *UUVD = dyn_cast<UnresolvedUsingValueDecl>(this)) {
ASTContext &Context = getASTContext();
- return Context.getCanonicalNestedNameSpecifier(
- cast<UnresolvedUsingValueDecl>(this)->getQualifier()) ==
+ return Context.getCanonicalNestedNameSpecifier(UUVD->getQualifier()) ==
Context.getCanonicalNestedNameSpecifier(
cast<UnresolvedUsingValueDecl>(OldD)->getQualifier());
}
- // A typedef of an Objective-C class type can replace an Objective-C class
- // declaration or definition, and vice versa.
- // FIXME: Is this correct if one of the decls comes from an inline namespace?
- if ((isa<TypedefNameDecl>(this) && isa<ObjCInterfaceDecl>(OldD)) ||
- (isa<ObjCInterfaceDecl>(this) && isa<TypedefNameDecl>(OldD)))
- return true;
+ // UsingDirectiveDecl's are not really NamedDecl's, and all have same name.
+ // We want to keep it, unless it nominates same namespace.
+ if (auto *UD = dyn_cast<UsingDirectiveDecl>(this))
+ return UD->getNominatedNamespace()->getOriginalNamespace() ==
+ cast<UsingDirectiveDecl>(OldD)->getNominatedNamespace()
+ ->getOriginalNamespace();
+
+ if (!IsKnownNewer && isRedeclarable(getKind())) {
+ // Check whether this is actually newer than OldD. We want to keep the
+ // newer declaration. This loop will usually only iterate once, because
+ // OldD is usually the previous declaration.
+ for (auto D : redecls()) {
+ if (D == OldD)
+ break;
+
+ // If we reach the canonical declaration, then OldD is not actually older
+ // than this one.
+ //
+ // FIXME: In this case, we should not add this decl to the lookup table.
+ if (D->isCanonicalDecl())
+ return false;
+ }
+ }
- // For non-function declarations, if the declarations are of the
- // same kind and have the same parent then this must be a redeclaration,
- // or semantic analysis would not have given us the new declaration.
- // Note that inline namespaces can give us two declarations with the same
- // name and kind in the same scope but different contexts.
- return this->getKind() == OldD->getKind() &&
- this->getDeclContext()->getRedeclContext()->Equals(
- OldD->getDeclContext()->getRedeclContext());
+ // It's a newer declaration of the same kind of declaration in the same scope,
+ // and not an overload: we want this decl instead of the existing one.
+ return true;
}
bool NamedDecl::hasLinkage() const {
Modified: cfe/trunk/lib/AST/DeclBase.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=228661&r1=228660&r2=228661&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclBase.cpp (original)
+++ cfe/trunk/lib/AST/DeclBase.cpp Mon Feb 9 21:28:10 2015
@@ -1082,7 +1082,7 @@ ExternalASTSource::SetExternalVisibleDec
// first.
llvm::SmallVector<unsigned, 8> Skip;
for (unsigned I = 0, N = Decls.size(); I != N; ++I)
- if (List.HandleRedeclaration(Decls[I]))
+ if (List.HandleRedeclaration(Decls[I], /*IsKnownNewer*/false))
Skip.push_back(I);
Skip.push_back(Decls.size());
@@ -1564,7 +1564,7 @@ void DeclContext::makeDeclVisibleInConte
return;
}
- if (DeclNameEntries.HandleRedeclaration(D)) {
+ if (DeclNameEntries.HandleRedeclaration(D, /*IsKnownNewer*/!Internal)) {
// This declaration has replaced an existing one for which
// declarationReplaces returns true.
return;
Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=228661&r1=228660&r2=228661&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Mon Feb 9 21:28:10 2015
@@ -414,6 +414,8 @@ void LookupResult::resolveKind() {
if (!Unique.insert(D).second) {
// If it's not unique, pull something off the back (and
// continue at this index).
+ // FIXME: This is wrong. We need to take the more recent declaration in
+ // order to get the right type, default arguments, etc.
Decls[I] = Decls[--N];
continue;
}
@@ -1254,6 +1256,9 @@ static NamedDecl *findAcceptableDecl(Sem
for (auto RD : D->redecls()) {
if (auto ND = dyn_cast<NamedDecl>(RD)) {
+ // FIXME: This is wrong in the case where the previous declaration is not
+ // visible in the same scope as D. This needs to be done much more
+ // carefully.
if (LookupResult::isVisible(SemaRef, ND))
return ND;
}
Modified: cfe/trunk/test/Modules/Inputs/cxx-lookup/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/cxx-lookup/module.modulemap?rev=228661&r1=228660&r2=228661&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/cxx-lookup/module.modulemap (original)
+++ cfe/trunk/test/Modules/Inputs/cxx-lookup/module.modulemap Mon Feb 9 21:28:10 2015
@@ -6,3 +6,5 @@ module C {
}
module X { header "x.h" export * }
module Y { header "y.h" export * }
+module na { header "na.h" export * }
+module nb { header "nb.h" export * }
Added: cfe/trunk/test/Modules/Inputs/cxx-lookup/na.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/cxx-lookup/na.h?rev=228661&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/cxx-lookup/na.h (added)
+++ cfe/trunk/test/Modules/Inputs/cxx-lookup/na.h Mon Feb 9 21:28:10 2015
@@ -0,0 +1 @@
+namespace N { struct S { friend struct foo; }; }
Added: cfe/trunk/test/Modules/Inputs/cxx-lookup/nb.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/cxx-lookup/nb.h?rev=228661&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/cxx-lookup/nb.h (added)
+++ cfe/trunk/test/Modules/Inputs/cxx-lookup/nb.h Mon Feb 9 21:28:10 2015
@@ -0,0 +1 @@
+namespace N { extern int n; }
Modified: cfe/trunk/test/Modules/cxx-lookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/cxx-lookup.cpp?rev=228661&r1=228660&r2=228661&view=diff
==============================================================================
--- cfe/trunk/test/Modules/cxx-lookup.cpp (original)
+++ cfe/trunk/test/Modules/cxx-lookup.cpp Mon Feb 9 21:28:10 2015
@@ -4,3 +4,8 @@
namespace llvm {}
#include "c2.h"
llvm::GlobalValue *p;
+
+#include "na.h"
+namespace N { struct foo; }
+#include "nb.h"
+N::foo *use_n_foo;
More information about the cfe-commits
mailing list