r251898 - [modules] Rationalize the behavior of Decl::declarationReplaces, and in
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 2 19:13:12 PST 2015
Author: rsmith
Date: Mon Nov 2 21:13:11 2015
New Revision: 251898
URL: http://llvm.org/viewvc/llvm-project?rev=251898&view=rev
Log:
[modules] Rationalize the behavior of Decl::declarationReplaces, and in
particular don't assume that two declarations of the same kind in the same
context are declaring the same entity. That's not true when the same name is
declared multiple times as internal-linkage symbols within a module.
(getCanonicalDecl is cheap now, so we can just use it here.)
Added:
cfe/trunk/test/Modules/Inputs/internal-constants/
cfe/trunk/test/Modules/Inputs/internal-constants/a.h
cfe/trunk/test/Modules/Inputs/internal-constants/b.h
cfe/trunk/test/Modules/Inputs/internal-constants/c.h
cfe/trunk/test/Modules/Inputs/internal-constants/const.h
cfe/trunk/test/Modules/Inputs/internal-constants/module.modulemap
cfe/trunk/test/Modules/internal-constants.cpp
Modified:
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
cfe/trunk/test/SemaCXX/cxx1y-variable-templates_top_level.cpp
cfe/trunk/test/SemaCXX/enable_if.cpp
Modified: cfe/trunk/lib/AST/Decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=251898&r1=251897&r2=251898&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Mon Nov 2 21:13:11 2015
@@ -1451,32 +1451,6 @@ void NamedDecl::getNameForDiagnostic(raw
printName(OS);
}
-static bool isKindReplaceableBy(Decl::Kind OldK, Decl::Kind NewK) {
- // For method declarations, we never replace.
- if (ObjCMethodDecl::classofKind(NewK))
- return false;
-
- 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;
}
@@ -1500,9 +1474,19 @@ bool NamedDecl::declarationReplaces(Name
if (OldD->isFromASTFile() && isFromASTFile())
return false;
- if (!isKindReplaceableBy(OldD->getKind(), getKind()))
+ // A kind mismatch implies that the declaration is not replaced.
+ if (OldD->getKind() != getKind())
+ return false;
+
+ // For method declarations, we never replace. (Why?)
+ if (isa<ObjCMethodDecl>(this))
return false;
+ // For parameters, pick the newer one. This is either an error or (in
+ // Objective-C) permitted as an extension.
+ if (isa<ParmVarDecl>(this))
+ return true;
+
// 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.
@@ -1510,28 +1494,8 @@ bool NamedDecl::declarationReplaces(Name
OldD->getDeclContext()->getRedeclContext()))
return false;
- 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;
-
- // 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;
-
- // Using declarations can be overloaded if they introduce functions.
+ // Using declarations can be replaced if they import the same name from the
+ // same context.
if (auto *UD = dyn_cast<UsingDecl>(this)) {
ASTContext &Context = getASTContext();
return Context.getCanonicalNestedNameSpecifier(UD->getQualifier()) ==
@@ -1546,13 +1510,20 @@ bool NamedDecl::declarationReplaces(Name
}
// UsingDirectiveDecl's are not really NamedDecl's, and all have same name.
- // We want to keep it, unless it nominates same namespace.
+ // They can be replaced if they nominate the same namespace.
+ // FIXME: Is this true even if they have different module visibility?
if (auto *UD = dyn_cast<UsingDirectiveDecl>(this))
return UD->getNominatedNamespace()->getOriginalNamespace() ==
cast<UsingDirectiveDecl>(OldD)->getNominatedNamespace()
->getOriginalNamespace();
- if (!IsKnownNewer && isRedeclarable(getKind())) {
+ if (isRedeclarable(getKind())) {
+ if (getCanonicalDecl() != OldD->getCanonicalDecl())
+ return false;
+
+ if (IsKnownNewer)
+ return true;
+
// 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.
@@ -1567,11 +1538,16 @@ bool NamedDecl::declarationReplaces(Name
if (D->isCanonicalDecl())
return false;
}
+
+ // It's a newer declaration of the same kind of declaration in the same
+ // scope: we want this decl instead of the existing one.
+ return true;
}
- // 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;
+ // In all other cases, we need to keep both declarations in case they have
+ // different visibility. Any attempt to use the name will result in an
+ // ambiguity if more than one is visible.
+ return false;
}
bool NamedDecl::hasLinkage() const {
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=251898&r1=251897&r2=251898&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Nov 2 21:13:11 2015
@@ -3352,7 +3352,7 @@ void Sema::MergeVarDecl(VarDecl *New, Lo
!TemplateParameterListsAreEqual(NewTemplate->getTemplateParameters(),
OldTemplate->getTemplateParameters(),
/*Complain=*/true, TPL_TemplateMatch))
- return;
+ return New->setInvalidDecl();
// C++ [class.mem]p1:
// A member shall not be declared twice in the member-specification [...]
@@ -8220,7 +8220,7 @@ bool Sema::CheckFunctionDeclaration(Scop
// there's no more work to do here; we'll just add the new
// function to the scope.
if (!AllowOverloadingOfFunction(Previous, Context)) {
- NamedDecl *Candidate = Previous.getFoundDecl();
+ NamedDecl *Candidate = Previous.getRepresentativeDecl();
if (shouldLinkPossiblyHiddenDecl(Candidate, NewFD)) {
Redeclaration = true;
OldDecl = Candidate;
Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=251898&r1=251897&r2=251898&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Mon Nov 2 21:13:11 2015
@@ -474,7 +474,7 @@ void LookupResult::resolveKind() {
D = cast<NamedDecl>(D->getCanonicalDecl());
// Ignore an invalid declaration unless it's the only one left.
- if (D->isInvalidDecl() && I < N-1) {
+ if (D->isInvalidDecl() && !(I == 0 && N == 1)) {
Decls[I] = Decls[--N];
continue;
}
Added: cfe/trunk/test/Modules/Inputs/internal-constants/a.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/internal-constants/a.h?rev=251898&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/internal-constants/a.h (added)
+++ cfe/trunk/test/Modules/Inputs/internal-constants/a.h Mon Nov 2 21:13:11 2015
@@ -0,0 +1,3 @@
+#pragma once
+#include "const.h"
+inline int f() { return N::k; }
Added: cfe/trunk/test/Modules/Inputs/internal-constants/b.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/internal-constants/b.h?rev=251898&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/internal-constants/b.h (added)
+++ cfe/trunk/test/Modules/Inputs/internal-constants/b.h Mon Nov 2 21:13:11 2015
@@ -0,0 +1,3 @@
+#pragma once
+#include "const.h"
+inline int g() { return N::k; }
Added: cfe/trunk/test/Modules/Inputs/internal-constants/c.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/internal-constants/c.h?rev=251898&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/internal-constants/c.h (added)
+++ cfe/trunk/test/Modules/Inputs/internal-constants/c.h Mon Nov 2 21:13:11 2015
@@ -0,0 +1,3 @@
+#pragma once
+#include "a.h"
+inline int h() { return N::k; }
Added: cfe/trunk/test/Modules/Inputs/internal-constants/const.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/internal-constants/const.h?rev=251898&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/internal-constants/const.h (added)
+++ cfe/trunk/test/Modules/Inputs/internal-constants/const.h Mon Nov 2 21:13:11 2015
@@ -0,0 +1,3 @@
+namespace N {
+ const int k = 5;
+}
Added: cfe/trunk/test/Modules/Inputs/internal-constants/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/internal-constants/module.modulemap?rev=251898&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/internal-constants/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/internal-constants/module.modulemap Mon Nov 2 21:13:11 2015
@@ -0,0 +1,6 @@
+module X {
+ textual header "const.h"
+ module A { header "a.h" export * }
+ module B { header "b.h" export * }
+ module C { header "c.h" export * }
+}
Added: cfe/trunk/test/Modules/internal-constants.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/internal-constants.cpp?rev=251898&view=auto
==============================================================================
--- cfe/trunk/test/Modules/internal-constants.cpp (added)
+++ cfe/trunk/test/Modules/internal-constants.cpp Mon Nov 2 21:13:11 2015
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fmodules-local-submodule-visibility -I%S/Inputs/internal-constants %s -verify
+
+// expected-no-diagnostics
+#include "c.h"
+
+int q = h();
+int r = N::k;
+
+#include "b.h"
+
+int s = N::k; // FIXME: This should be ambiguous if we really want internal linkage declarations to not collide.
Modified: cfe/trunk/test/SemaCXX/cxx1y-variable-templates_top_level.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx1y-variable-templates_top_level.cpp?rev=251898&r1=251897&r2=251898&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/cxx1y-variable-templates_top_level.cpp (original)
+++ cfe/trunk/test/SemaCXX/cxx1y-variable-templates_top_level.cpp Mon Nov 2 21:13:11 2015
@@ -90,8 +90,8 @@ namespace odr_tmpl {
}
namespace pvt_diff_params {
- template<typename T, typename> T v; // expected-note {{previous template declaration is here}}
- template<typename T> T v; // expected-error {{too few template parameters in template redeclaration}} expected-note {{previous template declaration is here}}
+ template<typename T, typename> T v; // expected-note 2{{previous template declaration is here}}
+ template<typename T> T v; // expected-error {{too few template parameters in template redeclaration}}
template<typename T, typename, typename> T v; // expected-error {{too many template parameters in template redeclaration}}
}
Modified: cfe/trunk/test/SemaCXX/enable_if.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/enable_if.cpp?rev=251898&r1=251897&r2=251898&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/enable_if.cpp (original)
+++ cfe/trunk/test/SemaCXX/enable_if.cpp Mon Nov 2 21:13:11 2015
@@ -11,7 +11,7 @@ struct X {
X(bool b) __attribute__((enable_if(b, "chosen when 'b' is true"))); // expected-note{{candidate disabled: chosen when 'b' is true}}
void f(int n) __attribute__((enable_if(n == 0, "chosen when 'n' is zero")));
- void f(int n) __attribute__((enable_if(n == 1, "chosen when 'n' is one"))); // expected-note{{member declaration nearly matches}} expected-note{{candidate disabled: chosen when 'n' is one}}
+ void f(int n) __attribute__((enable_if(n == 1, "chosen when 'n' is one"))); // expected-note{{member declaration nearly matches}} expected-note 2{{candidate disabled: chosen when 'n' is one}}
void g(int n) __attribute__((enable_if(n == 0, "chosen when 'n' is zero"))); // expected-note{{candidate disabled: chosen when 'n' is zero}}
@@ -31,11 +31,11 @@ struct X {
operator fp() __attribute__((enable_if(false, "never enabled"))) { return surrogate; } // expected-note{{conversion candidate of type 'int (*)(int)'}} // FIXME: the message is not displayed
};
-void X::f(int n) __attribute__((enable_if(n == 0, "chosen when 'n' is zero"))) // expected-note{{member declaration nearly matches}} expected-note{{candidate disabled: chosen when 'n' is zero}}
+void X::f(int n) __attribute__((enable_if(n == 0, "chosen when 'n' is zero"))) // expected-note{{member declaration nearly matches}} expected-note 2{{candidate disabled: chosen when 'n' is zero}}
{
}
-void X::f(int n) __attribute__((enable_if(n == 2, "chosen when 'n' is two"))) // expected-error{{out-of-line definition of 'f' does not match any declaration in 'X'}} expected-note{{candidate disabled: chosen when 'n' is two}}
+void X::f(int n) __attribute__((enable_if(n == 2, "chosen when 'n' is two"))) // expected-error{{out-of-line definition of 'f' does not match any declaration in 'X'}}
{
}
@@ -73,7 +73,7 @@ void test() {
X x;
x.f(0);
x.f(1);
- x.f(2); // no error, suppressed by erroneous out-of-line definition
+ x.f(2); // expected-error{{no matching member function for call to 'f'}}
x.f(3); // expected-error{{no matching member function for call to 'f'}}
x.g(0);
More information about the cfe-commits
mailing list