r252960 - [modules] Simplify and generalize the existing rule for finding hidden
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 12 14:19:46 PST 2015
Author: rsmith
Date: Thu Nov 12 16:19:45 2015
New Revision: 252960
URL: http://llvm.org/viewvc/llvm-project?rev=252960&view=rev
Log:
[modules] Simplify and generalize the existing rule for finding hidden
declarations in redeclaration lookup. A declaration is now visible to
lookup if:
* It is visible (not in a module, or in an imported module), or
* We're doing redeclaration lookup and it's externally-visible, or
* We're doing typo correction and looking for unimported decls.
We now support multiple modules having different internal-linkage or no-linkage
definitions of the same name for all entities, not just for functions,
variables, and some typedefs. As previously, if multiple such entities are
visible, any attempt to use them will result in an ambiguity error.
This patch fixes the linkage calculation for a number of entities where we
previously didn't need to get it right (using-declarations, namespace aliases,
and so on). It also classifies enumerators as always having no linkage, which
is a slight deviation from the C++ standard's definition, but not an observable
change outside modules (this change is being discussed on the -core reflector
currently).
This also removes the prior special case for tag lookup, which made some cases
of this work, but also led to bizarre, bogus "must use 'struct' to refer to type
'Foo' in this scope" diagnostics in C++.
Added:
cfe/trunk/test/Modules/Inputs/no-linkage/
cfe/trunk/test/Modules/Inputs/no-linkage/decls.h
cfe/trunk/test/Modules/Inputs/no-linkage/empty.h
cfe/trunk/test/Modules/Inputs/no-linkage/module.modulemap
cfe/trunk/test/Modules/no-linkage.cpp
Modified:
cfe/trunk/include/clang/Sema/Lookup.h
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/Index/linkage.c
cfe/trunk/test/Index/usrs.m
cfe/trunk/test/Modules/decldef.m
cfe/trunk/test/Modules/merge-enumerators.cpp
cfe/trunk/test/Modules/module-private.cpp
cfe/trunk/test/Modules/submodule-visibility-cycles.cpp
cfe/trunk/test/Modules/submodules-merge-defs.cpp
Modified: cfe/trunk/include/clang/Sema/Lookup.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Lookup.h?rev=252960&r1=252959&r2=252960&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Lookup.h (original)
+++ cfe/trunk/include/clang/Sema/Lookup.h Thu Nov 12 16:19:45 2015
@@ -139,8 +139,7 @@ public:
Redecl(Redecl != Sema::NotForRedeclaration),
HideTags(true),
Diagnose(Redecl == Sema::NotForRedeclaration),
- AllowHidden(Redecl == Sema::ForRedeclaration),
- AllowHiddenInternal(AllowHidden),
+ AllowHidden(false),
Shadowed(false)
{
configure();
@@ -162,8 +161,7 @@ public:
Redecl(Redecl != Sema::NotForRedeclaration),
HideTags(true),
Diagnose(Redecl == Sema::NotForRedeclaration),
- AllowHidden(Redecl == Sema::ForRedeclaration),
- AllowHiddenInternal(AllowHidden),
+ AllowHidden(false),
Shadowed(false)
{
configure();
@@ -184,7 +182,6 @@ public:
HideTags(Other.HideTags),
Diagnose(false),
AllowHidden(Other.AllowHidden),
- AllowHiddenInternal(Other.AllowHiddenInternal),
Shadowed(false)
{}
@@ -226,27 +223,16 @@ public:
/// \brief Specify whether hidden declarations are visible, e.g.,
/// for recovery reasons.
void setAllowHidden(bool AH) {
- AllowHiddenInternal = AllowHidden = AH;
- }
-
- /// \brief Specify whether hidden internal declarations are visible.
- void setAllowHiddenInternal(bool AHI) {
- AllowHiddenInternal = AHI;
+ AllowHidden = AH;
}
/// \brief Determine whether this lookup is permitted to see hidden
/// declarations, such as those in modules that have not yet been imported.
bool isHiddenDeclarationVisible(NamedDecl *ND) const {
- // If a using-shadow declaration is hidden, it's never visible, not
- // even to redeclaration lookup.
- // FIXME: Should this apply to typedefs and namespace aliases too?
- if (isa<UsingShadowDecl>(ND) && LookupKind != Sema::LookupUsingDeclName)
- return false;
- return (AllowHidden &&
- (AllowHiddenInternal || ND->isExternallyVisible())) ||
- LookupKind == Sema::LookupTagName;
+ return AllowHidden ||
+ (isForRedeclaration() && ND->isExternallyVisible());
}
-
+
/// Sets whether tag declarations should be hidden by non-tag
/// declarations during resolution. The default is true.
void setHideTags(bool Hide) {
@@ -317,7 +303,8 @@ public:
if (!D->isInIdentifierNamespace(IDNS))
return nullptr;
- if (isHiddenDeclarationVisible(D) || isVisible(getSema(), D))
+ if (!D->isHidden() || isHiddenDeclarationVisible(D) ||
+ isVisibleSlow(getSema(), D))
return D;
return getAcceptableDeclSlow(D);
@@ -526,7 +513,6 @@ public:
/// \brief Change this lookup's redeclaration kind.
void setRedeclarationKind(Sema::RedeclarationKind RK) {
Redecl = RK;
- AllowHiddenInternal = AllowHidden = (RK == Sema::ForRedeclaration);
configure();
}
@@ -698,9 +684,6 @@ private:
/// \brief True if we should allow hidden declarations to be 'visible'.
bool AllowHidden;
- /// \brief True if we should allow hidden internal declarations to be visible.
- bool AllowHiddenInternal;
-
/// \brief True if the found declarations were shadowed by some other
/// declaration that we skipped. This only happens when \c LookupKind
/// is \c LookupRedeclarationWithLinkage.
Modified: cfe/trunk/lib/AST/Decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=252960&r1=252959&r2=252960&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Thu Nov 12 16:19:45 2015
@@ -635,6 +635,8 @@ static LinkageInfo getLVForNamespaceScop
if (D->isInAnonymousNamespace()) {
const auto *Var = dyn_cast<VarDecl>(D);
const auto *Func = dyn_cast<FunctionDecl>(D);
+ // FIXME: In C++11 onwards, anonymous namespaces should give decls
+ // within them internal linkage, not unique external linkage.
if ((!Var || !isFirstInExternCContext(Var)) &&
(!Func || !isFirstInExternCContext(Func)))
return LinkageInfo::uniqueExternal();
@@ -821,10 +823,14 @@ static LinkageInfo getLVForNamespaceScop
} else if (isa<ObjCInterfaceDecl>(D)) {
// fallout
+ } else if (auto *TD = dyn_cast<TypedefNameDecl>(D)) {
+ // A typedef declaration has linkage if it gives a type a name for
+ // linkage purposes.
+ if (!TD->getAnonDeclWithTypedefName(/*AnyRedecl*/true))
+ return LinkageInfo::none();
+
// Everything not covered here has no linkage.
} else {
- // FIXME: A typedef declaration has linkage if it gives a type a name for
- // linkage purposes.
return LinkageInfo::none();
}
@@ -1226,8 +1232,32 @@ static LinkageInfo computeLVForDecl(cons
switch (D->getKind()) {
default:
break;
+
+ // Per C++ [basic.link]p2, only the names of objects, references,
+ // functions, types, templates, namespaces, and values ever have linkage.
+ //
+ // Note that the name of a typedef, namespace alias, using declaration,
+ // and so on are not the name of the corresponding type, namespace, or
+ // declaration, so they do *not* have linkage.
+ case Decl::EnumConstant: // FIXME: This has linkage, but that's dumb.
+ case Decl::ImplicitParam:
+ case Decl::Label:
+ case Decl::NamespaceAlias:
case Decl::ParmVar:
+ case Decl::Using:
+ case Decl::UsingShadow:
+ case Decl::UsingDirective:
return LinkageInfo::none();
+
+ case Decl::Typedef:
+ case Decl::TypeAlias:
+ // A typedef declaration has linkage if it gives a type a name for
+ // linkage purposes.
+ if (!cast<TypedefNameDecl>(D)
+ ->getAnonDeclWithTypedefName(/*AnyRedecl*/true))
+ return LinkageInfo::none();
+ break;
+
case Decl::TemplateTemplateParm: // count these as external
case Decl::NonTypeTemplateParm:
case Decl::ObjCAtDefsField:
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=252960&r1=252959&r2=252960&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Nov 12 16:19:45 2015
@@ -4819,12 +4819,6 @@ NamedDecl *Sema::HandleDeclarator(Scope
LookupResult Previous(*this, NameInfo, LookupOrdinaryName,
ForRedeclaration);
- // If we're hiding internal-linkage symbols in modules from redeclaration
- // lookup, let name lookup know.
- if ((getLangOpts().Modules || getLangOpts().ModulesLocalVisibility) &&
- D.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_typedef)
- Previous.setAllowHiddenInternal(false);
-
// See if this is a redefinition of a variable in the same scope.
if (!D.getCXXScopeSpec().isSet()) {
bool IsLinkageLookup = false;
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=252960&r1=252959&r2=252960&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Nov 12 16:19:45 2015
@@ -7206,23 +7206,13 @@ Decl *Sema::ActOnStartNamespaceDef(Scope
// treated as an original-namespace-name.
//
// Since namespace names are unique in their scope, and we don't
- // look through using directives, just look for any ordinary names.
-
- const unsigned IDNS = Decl::IDNS_Ordinary | Decl::IDNS_Member |
- Decl::IDNS_Type | Decl::IDNS_Using | Decl::IDNS_Tag |
- Decl::IDNS_Namespace;
- NamedDecl *PrevDecl = nullptr;
- DeclContext::lookup_result R = CurContext->getRedeclContext()->lookup(II);
- for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); I != E;
- ++I) {
- if ((*I)->getIdentifierNamespace() & IDNS) {
- PrevDecl = *I;
- break;
- }
- }
-
+ // look through using directives, just look for any ordinary names
+ // as if by qualified name lookup.
+ LookupResult R(*this, II, IdentLoc, LookupOrdinaryName, ForRedeclaration);
+ LookupQualifiedName(R, CurContext->getRedeclContext());
+ NamedDecl *PrevDecl = R.getAsSingle<NamedDecl>();
PrevNS = dyn_cast_or_null<NamespaceDecl>(PrevDecl);
-
+
if (PrevNS) {
// This is an extended namespace definition.
if (IsInline != PrevNS->isInline())
Modified: cfe/trunk/test/Index/linkage.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/linkage.c?rev=252960&r1=252959&r2=252960&view=diff
==============================================================================
--- cfe/trunk/test/Index/linkage.c (original)
+++ cfe/trunk/test/Index/linkage.c Thu Nov 12 16:19:45 2015
@@ -20,7 +20,7 @@ void f16(void) {
// CHECK: EnumDecl=Baz:3:6 (Definition)linkage=External
-// CHECK: EnumConstantDecl=Qux:3:12 (Definition)linkage=External
+// CHECK: EnumConstantDecl=Qux:3:12 (Definition)linkage=NoLinkage
// CHECK: VarDecl=x:4:5linkage=External
// CHECK: FunctionDecl=foo:5:6linkage=External
// CHECK: VarDecl=w:6:12linkage=Internal
Modified: cfe/trunk/test/Index/usrs.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/usrs.m?rev=252960&r1=252959&r2=252960&view=diff
==============================================================================
--- cfe/trunk/test/Index/usrs.m (original)
+++ cfe/trunk/test/Index/usrs.m Thu Nov 12 16:19:45 2015
@@ -119,7 +119,7 @@ int test_multi_declaration(void) {
// CHECK: usrs.m c:@SA at MyStruct Extent=[15:9 - 18:2]
// CHECK: usrs.m c:@SA at MyStruct@FI at wa Extent=[16:3 - 16:9]
// CHECK: usrs.m c:@SA at MyStruct@FI at moo Extent=[17:3 - 17:10]
-// CHECK: usrs.m c:usrs.m at T@MyStruct Extent=[15:1 - 18:11]
+// CHECK: usrs.m c:@T at MyStruct Extent=[15:1 - 18:11]
// CHECK: usrs.m c:@E at Pizza Extent=[20:1 - 23:2]
// CHECK: usrs.m c:@E at Pizza@CHEESE Extent=[21:3 - 21:9]
// CHECK: usrs.m c:@E at Pizza@MUSHROOMS Extent=[22:3 - 22:12]
Added: cfe/trunk/test/Modules/Inputs/no-linkage/decls.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/no-linkage/decls.h?rev=252960&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/no-linkage/decls.h (added)
+++ cfe/trunk/test/Modules/Inputs/no-linkage/decls.h Thu Nov 12 16:19:45 2015
@@ -0,0 +1,6 @@
+namespace RealNS { int UsingDecl; }
+namespace NS = RealNS;
+typedef int Typedef;
+using AliasDecl = int;
+enum Enum { Enumerator };
+using RealNS::UsingDecl;
Added: cfe/trunk/test/Modules/Inputs/no-linkage/empty.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/no-linkage/empty.h?rev=252960&view=auto
==============================================================================
(empty)
Added: cfe/trunk/test/Modules/Inputs/no-linkage/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/no-linkage/module.modulemap?rev=252960&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/no-linkage/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/no-linkage/module.modulemap Thu Nov 12 16:19:45 2015
@@ -0,0 +1 @@
+module M { module E { header "empty.h" } module D { header "decls.h" } }
Modified: cfe/trunk/test/Modules/decldef.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/decldef.m?rev=252960&r1=252959&r2=252960&view=diff
==============================================================================
--- cfe/trunk/test/Modules/decldef.m (original)
+++ cfe/trunk/test/Modules/decldef.m Thu Nov 12 16:19:45 2015
@@ -11,7 +11,13 @@ Def *def;
#ifdef USE_EARLY
A *a1; // expected-error{{declaration of 'A' must be imported from module 'decldef.Def' before it is required}}
#endif
-B *b1; // expected-error{{must use 'struct' tag to refer to type 'B'}}
+B *b1;
+#ifdef USE_EARLY
+// expected-error at -2{{must use 'struct' tag to refer to type 'B'}}
+#else
+// expected-error at -4{{declaration of 'B' must be imported from module 'decldef.Decl' before it is required}}
+// expected-note at Inputs/decl.h:2 {{previous}}
+#endif
@import decldef.Decl;
A *a2;
Modified: cfe/trunk/test/Modules/merge-enumerators.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-enumerators.cpp?rev=252960&r1=252959&r2=252960&view=diff
==============================================================================
--- cfe/trunk/test/Modules/merge-enumerators.cpp (original)
+++ cfe/trunk/test/Modules/merge-enumerators.cpp Thu Nov 12 16:19:45 2015
@@ -16,11 +16,13 @@
#ifdef MERGE_LATE
namespace N {
- // FIXME: Should we accept this and reject the usage below due to ambiguity instead?
- enum { A } a; // expected-error {{redefinition of enumerator 'A'}}
- // expected-note at a.h:1 {{here}} (from module B.b)
+ enum { A } a; // expected-note {{candidate}}
+ // expected-note at a.h:1 {{candidate}} (from module B.b)
}
#include "a.h"
#endif
N::E e = N::A;
+#ifdef MERGE_LATE
+// expected-error at -2 {{ambiguous}}
+#endif
Modified: cfe/trunk/test/Modules/module-private.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/module-private.cpp?rev=252960&r1=252959&r2=252960&view=diff
==============================================================================
--- cfe/trunk/test/Modules/module-private.cpp (original)
+++ cfe/trunk/test/Modules/module-private.cpp Thu Nov 12 16:19:45 2015
@@ -12,11 +12,7 @@ void test() {
}
int test_broken() {
- HiddenStruct hidden; // \
- // expected-error{{must use 'struct' tag to refer to type 'HiddenStruct' in this scope}} \
- // expected-error{{definition of 'HiddenStruct' must be imported}}
- // expected-note at Inputs/module_private_left.h:3 {{previous definition is here}}
-
+ HiddenStruct hidden; // expected-error{{unknown type name 'HiddenStruct'}}
Integer i; // expected-error{{unknown type name 'Integer'}}
int *ip = 0;
Added: cfe/trunk/test/Modules/no-linkage.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/no-linkage.cpp?rev=252960&view=auto
==============================================================================
--- cfe/trunk/test/Modules/no-linkage.cpp (added)
+++ cfe/trunk/test/Modules/no-linkage.cpp Thu Nov 12 16:19:45 2015
@@ -0,0 +1,35 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fmodules-local-submodule-visibility -I%S/Inputs/no-linkage -fmodule-map-file=%S/Inputs/no-linkage/module.modulemap %s -verify
+
+#include "empty.h"
+
+namespace NS { int n; } // expected-note {{candidate}}
+struct Typedef { int n; }; // expected-note {{candidate}}
+int AliasDecl; // expected-note {{candidate}}
+enum AlsoAnEnum { Enumerator }; // expected-note {{candidate}}
+int UsingDecl; // expected-note {{candidate}}
+
+// expected-note at decls.h:2 {{candidate}}
+// expected-note at decls.h:3 {{candidate}}
+// expected-note at decls.h:4 {{candidate}}
+// expected-note at decls.h:5 {{candidate}}
+// expected-note at decls.h:6 {{candidate}}
+
+void use(int);
+void use_things() {
+ use(Typedef().n);
+ use(NS::n);
+ use(AliasDecl);
+ use(Enumerator);
+ use(UsingDecl);
+}
+
+#include "decls.h"
+
+void use_things_again() {
+ use(Typedef().n); // expected-error {{ambiguous}}
+ use(NS::n); // expected-error {{ambiguous}} expected-error{{'NS' is not a class, namespace, or enumeration}}
+ use(AliasDecl); // expected-error {{ambiguous}}
+ use(Enumerator); // expected-error {{ambiguous}}
+ use(UsingDecl); // expected-error {{ambiguous}}
+}
Modified: cfe/trunk/test/Modules/submodule-visibility-cycles.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/submodule-visibility-cycles.cpp?rev=252960&r1=252959&r2=252960&view=diff
==============================================================================
--- cfe/trunk/test/Modules/submodule-visibility-cycles.cpp (original)
+++ cfe/trunk/test/Modules/submodule-visibility-cycles.cpp Thu Nov 12 16:19:45 2015
@@ -3,7 +3,7 @@
#include "cycle1.h"
C1 c1;
-C2 c2; // expected-error {{must be imported}} expected-error {{}}
+C2 c2; // expected-error {{must be imported}}
// expected-note at cycle2.h:6 {{here}}
#include "cycle2.h"
Modified: cfe/trunk/test/Modules/submodules-merge-defs.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/submodules-merge-defs.cpp?rev=252960&r1=252959&r2=252960&view=diff
==============================================================================
--- cfe/trunk/test/Modules/submodules-merge-defs.cpp (original)
+++ cfe/trunk/test/Modules/submodules-merge-defs.cpp Thu Nov 12 16:19:45 2015
@@ -12,7 +12,7 @@
#include "indirect.h"
#endif
-A pre_a; // expected-error {{must use 'struct'}}
+A pre_a;
#ifdef IMPORT_USE_2
// expected-error-re at -2 {{must be imported from one of {{.*}}stuff.use{{.*}}stuff.use-2}}
#elif EARLY_INDIRECT_INCLUDE
@@ -21,29 +21,28 @@ A pre_a; // expected-error {{must use 's
// expected-error at -6 {{must be imported from module 'stuff.use'}}
#endif
// expected-note at defs.h:1 +{{here}}
+extern class A pre_a2;
+int pre_use_a = use_a(pre_a2); // expected-error {{'A' must be imported}} expected-error {{'use_a' must be imported}}
// expected-note at defs.h:2 +{{here}}
-int pre_use_a = use_a(pre_a); // expected-error {{'A' must be imported}} expected-error {{'use_a' must be imported}}
B::Inner2 pre_bi; // expected-error +{{must be imported}}
// expected-note at defs.h:4 +{{here}}
// expected-note at defs.h:17 +{{here}}
-void pre_bfi(B b) { // expected-error {{must use 'class'}} expected-error +{{must be imported}}
- b.f<int>(); // expected-error +{{must be imported}} expected-error +{{}}
- // expected-note at defs.h:19 +{{here}}
+void pre_bfi(B b) { // expected-error +{{must be imported}}
+ b.f<int>(); // expected-error +{{}}
}
C_Base<1> pre_cb1; // expected-error +{{must be imported}}
// expected-note at defs.h:23 +{{here}}
-C1 pre_c1; // expected-error +{{must be imported}} expected-error {{must use 'struct'}}
+C1 pre_c1; // expected-error +{{must be imported}}
// expected-note at defs.h:25 +{{here}}
-C2 pre_c2; // expected-error +{{must be imported}} expected-error {{must use 'struct'}}
+C2 pre_c2; // expected-error +{{must be imported}}
// expected-note at defs.h:26 +{{here}}
D::X pre_dx; // expected-error +{{must be imported}}
// expected-note at defs.h:28 +{{here}}
// expected-note at defs.h:29 +{{here}}
-// FIXME: We should warn that use_dx is being used without being imported.
-int pre_use_dx = use_dx(pre_dx);
+int pre_use_dx = use_dx(pre_dx); // ignored; pre_dx is invalid
int pre_e = E(0); // expected-error {{must be imported}}
// expected-note at defs.h:32 +{{here}}
@@ -69,8 +68,9 @@ J<> pre_j; // expected-error {{declarati
#endif
// expected-note at defs.h:58 +{{here}}
-ScopedEnum pre_scopedenum; // expected-error {{must be imported}} expected-error {{must use 'enum'}}
-// expected-note at defs.h:106 {{here}}
+ScopedEnum pre_scopedenum; // expected-error {{must be imported}}
+// expected-note at defs.h:105 0-1{{here}}
+// expected-note at defs.h:106 0-1{{here}}
enum ScopedEnum : int;
ScopedEnum pre_scopedenum_declared; // ok
More information about the cfe-commits
mailing list