From: Richard Smith Date: Tue, 9 Jun 2015 00:35:49 +0000 (+0000) Subject: [modules] Fix some visibility issues with default template arguments. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=3d5c8f8827bf601124907a8c69a37e2b412eced8;p=clang [modules] Fix some visibility issues with default template arguments. There are still problems here, but this is a better starting point. The main part of the change is: when doing a lookup that would accept visible or hidden declarations, prefer to produce the latest visible declaration if there are any visible declarations, rather than always producing the latest declaration. Thus, when we inherit default arguments (and other properties) from a previous declaration, we inherit them from the previous visible declaration; if the previous declaration is hidden, we already suppress inheritance of default arguments. There are a couple of other changes here that fix latent bugs exposed by this change. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@239371 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h index 451f9da1b6..e06b58b37b 100644 --- a/include/clang/AST/Decl.h +++ b/include/clang/AST/Decl.h @@ -236,7 +236,11 @@ public: bool isHidden() const { return Hidden; } /// \brief Set whether this declaration is hidden from name lookup. - void setHidden(bool Hide) { Hidden = Hide; } + void setHidden(bool Hide) { + assert((!Hide || isFromASTFile() || hasLocalOwningModuleStorage()) && + "declaration with no owning module can't be hidden"); + Hidden = Hide; + } /// \brief Determine whether this declaration is a C++ class member. bool isCXXClassMember() const { diff --git a/include/clang/AST/DeclBase.h b/include/clang/AST/DeclBase.h index 5c382b0d57..f176e5479e 100644 --- a/include/clang/AST/DeclBase.h +++ b/include/clang/AST/DeclBase.h @@ -637,6 +637,8 @@ public: private: Module *getOwningModuleSlow() const; +protected: + bool hasLocalOwningModuleStorage() const; public: /// \brief Get the imported owning module, if this decl is from an imported @@ -656,7 +658,7 @@ public: return reinterpret_cast(this)[-1]; } void setLocalOwningModule(Module *M) { - assert(!isFromASTFile() && Hidden && + assert(!isFromASTFile() && Hidden && hasLocalOwningModuleStorage() && "should not have a cached owning module"); reinterpret_cast(this)[-1] = M; } diff --git a/include/clang/Sema/Lookup.h b/include/clang/Sema/Lookup.h index 5bfee8b0d0..97192b53fa 100644 --- a/include/clang/Sema/Lookup.h +++ b/include/clang/Sema/Lookup.h @@ -302,10 +302,14 @@ public: if (!D->isInIdentifierNamespace(IDNS)) return nullptr; - if (isHiddenDeclarationVisible() || isVisible(getSema(), D)) + if (isVisible(getSema(), D)) return D; - return getAcceptableDeclSlow(D); + if (auto *Visible = getAcceptableDeclSlow(D)) + return Visible; + + // Even if hidden declarations are visible, prefer a visible declaration. + return isHiddenDeclarationVisible() ? D : nullptr; } private: diff --git a/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp index 79cadcfcb1..70bd16ffdd 100644 --- a/lib/AST/DeclBase.cpp +++ b/lib/AST/DeclBase.cpp @@ -80,6 +80,10 @@ Module *Decl::getOwningModuleSlow() const { return getASTContext().getExternalSource()->getModule(getOwningModuleID()); } +bool Decl::hasLocalOwningModuleStorage() const { + return getASTContext().getLangOpts().ModulesLocalVisibility; +} + const char *Decl::getDeclKindName() const { switch (DeclKind) { default: llvm_unreachable("Declaration not in DeclNodes.inc!"); diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp index 2600e8e44d..d0a55b57c6 100644 --- a/lib/Sema/SemaLookup.cpp +++ b/lib/Sema/SemaLookup.cpp @@ -1180,6 +1180,16 @@ Module *Sema::getOwningModule(Decl *Entity) { assert(!Entity->isFromASTFile() && "hidden entity from AST file has no owning module"); + if (!getLangOpts().ModulesLocalVisibility) { + // If we're not tracking visibility locally, the only way a declaration + // can be hidden and local is if it's hidden because it's parent is (for + // instance, maybe this is a lazily-declared special member of an imported + // class). + auto *Parent = cast(Entity->getDeclContext()); + assert(Parent->isHidden() && "unexpectedly hidden decl"); + return getOwningModule(Parent); + } + // It's local and hidden; grab or compute its owning module. M = Entity->getLocalOwningModule(); if (M) @@ -1218,9 +1228,11 @@ Module *Sema::getOwningModule(Decl *Entity) { } void Sema::makeMergedDefinitionVisible(NamedDecl *ND, SourceLocation Loc) { - auto *M = PP.getModuleContainingLocation(Loc); - assert(M && "hidden definition not in any module"); - Context.mergeDefinitionIntoModule(ND, M); + if (auto *M = PP.getModuleContainingLocation(Loc)) + Context.mergeDefinitionIntoModule(ND, M); + else + // We're not building a module; just make the definition visible. + ND->setHidden(false); } /// \brief Find the module in which the given declaration was defined. diff --git a/test/Modules/Inputs/template-default-args/a.h b/test/Modules/Inputs/template-default-args/a.h new file mode 100644 index 0000000000..1ef1ea5907 --- /dev/null +++ b/test/Modules/Inputs/template-default-args/a.h @@ -0,0 +1,4 @@ +template struct A {}; +template struct B {}; +template struct C; +template struct D; diff --git a/test/Modules/Inputs/template-default-args/b.h b/test/Modules/Inputs/template-default-args/b.h new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/Modules/Inputs/template-default-args/module.modulemap b/test/Modules/Inputs/template-default-args/module.modulemap new file mode 100644 index 0000000000..6182e6b3ee --- /dev/null +++ b/test/Modules/Inputs/template-default-args/module.modulemap @@ -0,0 +1 @@ +module X { module A { header "a.h" } module B { header "b.h" } } diff --git a/test/Modules/template-default-args.cpp b/test/Modules/template-default-args.cpp new file mode 100644 index 0000000000..63187b8dc1 --- /dev/null +++ b/test/Modules/template-default-args.cpp @@ -0,0 +1,22 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules -verify -fmodules-cache-path=%t -I %S/Inputs/template-default-args -std=c++11 %s +// +// expected-no-diagnostics + +template struct A; +template struct B; +template struct C; +template struct D; + +#include "b.h" + +template struct A {}; +template struct B {}; +template struct B; +template struct C; +template struct D {}; + +A<> a; +B<> b; +extern C<> c; +D<> d;