diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 098298f0c596..7485b12614d3 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -1451,32 +1451,6 @@ void NamedDecl::getNameForDiagnostic(raw_ostream &OS, 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 static bool isRedeclarableImpl(Redeclarable *) { return true; } @@ -1500,9 +1474,19 @@ bool NamedDecl::declarationReplaces(NamedDecl *OldD, bool IsKnownNewer) const { 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(this)) + return false; + + // For parameters, pick the newer one. This is either an error or (in + // Objective-C) permitted as an extension. + if (isa(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(NamedDecl *OldD, bool IsKnownNewer) const { OldD->getDeclContext()->getRedeclContext())) return false; - if (const FunctionDecl *FD = dyn_cast(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(this)) - return FunctionTemplate->getTemplatedDecl()->declarationReplaces( - cast(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(this)) - if (USD->getTargetDecl() != cast(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(this)) { ASTContext &Context = getASTContext(); return Context.getCanonicalNestedNameSpecifier(UD->getQualifier()) == @@ -1546,13 +1510,20 @@ bool NamedDecl::declarationReplaces(NamedDecl *OldD, bool IsKnownNewer) const { } // 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(this)) return UD->getNominatedNamespace()->getOriginalNamespace() == cast(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(NamedDecl *OldD, bool IsKnownNewer) const { 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 { diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 622ff1ebbce5..e188094f90ab 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -3352,7 +3352,7 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) { !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(Scope *S, FunctionDecl *NewFD, // 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; diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp index 6fbe0da8c901..0ba34ea8ef25 100644 --- a/clang/lib/Sema/SemaLookup.cpp +++ b/clang/lib/Sema/SemaLookup.cpp @@ -474,7 +474,7 @@ void LookupResult::resolveKind() { D = cast(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; } diff --git a/clang/test/Modules/Inputs/internal-constants/a.h b/clang/test/Modules/Inputs/internal-constants/a.h new file mode 100644 index 000000000000..d2881381b75d --- /dev/null +++ b/clang/test/Modules/Inputs/internal-constants/a.h @@ -0,0 +1,3 @@ +#pragma once +#include "const.h" +inline int f() { return N::k; } diff --git a/clang/test/Modules/Inputs/internal-constants/b.h b/clang/test/Modules/Inputs/internal-constants/b.h new file mode 100644 index 000000000000..679603afa256 --- /dev/null +++ b/clang/test/Modules/Inputs/internal-constants/b.h @@ -0,0 +1,3 @@ +#pragma once +#include "const.h" +inline int g() { return N::k; } diff --git a/clang/test/Modules/Inputs/internal-constants/c.h b/clang/test/Modules/Inputs/internal-constants/c.h new file mode 100644 index 000000000000..43a37f831a4e --- /dev/null +++ b/clang/test/Modules/Inputs/internal-constants/c.h @@ -0,0 +1,3 @@ +#pragma once +#include "a.h" +inline int h() { return N::k; } diff --git a/clang/test/Modules/Inputs/internal-constants/const.h b/clang/test/Modules/Inputs/internal-constants/const.h new file mode 100644 index 000000000000..e2dc8e14571f --- /dev/null +++ b/clang/test/Modules/Inputs/internal-constants/const.h @@ -0,0 +1,3 @@ +namespace N { + const int k = 5; +} diff --git a/clang/test/Modules/Inputs/internal-constants/module.modulemap b/clang/test/Modules/Inputs/internal-constants/module.modulemap new file mode 100644 index 000000000000..6d471f5fc94f --- /dev/null +++ b/clang/test/Modules/Inputs/internal-constants/module.modulemap @@ -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 * } +} diff --git a/clang/test/Modules/internal-constants.cpp b/clang/test/Modules/internal-constants.cpp new file mode 100644 index 000000000000..f95e95cc8bbb --- /dev/null +++ b/clang/test/Modules/internal-constants.cpp @@ -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. diff --git a/clang/test/SemaCXX/cxx1y-variable-templates_top_level.cpp b/clang/test/SemaCXX/cxx1y-variable-templates_top_level.cpp index 145bc49fff1f..787868fae176 100644 --- a/clang/test/SemaCXX/cxx1y-variable-templates_top_level.cpp +++ b/clang/test/SemaCXX/cxx1y-variable-templates_top_level.cpp @@ -90,8 +90,8 @@ namespace odr_tmpl { } namespace pvt_diff_params { - template T v; // expected-note {{previous template declaration is here}} - template T v; // expected-error {{too few template parameters in template redeclaration}} expected-note {{previous template declaration is here}} + template T v; // expected-note 2{{previous template declaration is here}} + template T v; // expected-error {{too few template parameters in template redeclaration}} template T v; // expected-error {{too many template parameters in template redeclaration}} } diff --git a/clang/test/SemaCXX/enable_if.cpp b/clang/test/SemaCXX/enable_if.cpp index b04d9b4c9dfc..b32bcd01f420 100644 --- a/clang/test/SemaCXX/enable_if.cpp +++ b/clang/test/SemaCXX/enable_if.cpp @@ -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);