r239371 - [modules] Fix some visibility issues with default template arguments.
Richard Smith
richard-llvm at metafoo.co.uk
Mon Jun 8 17:35:50 PDT 2015
Author: rsmith
Date: Mon Jun 8 19:35:49 2015
New Revision: 239371
URL: http://llvm.org/viewvc/llvm-project?rev=239371&view=rev
Log:
[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.
Added:
cfe/trunk/test/Modules/Inputs/template-default-args/
cfe/trunk/test/Modules/Inputs/template-default-args/a.h
cfe/trunk/test/Modules/Inputs/template-default-args/b.h
cfe/trunk/test/Modules/Inputs/template-default-args/module.modulemap
cfe/trunk/test/Modules/template-default-args.cpp
Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/DeclBase.h
cfe/trunk/include/clang/Sema/Lookup.h
cfe/trunk/lib/AST/DeclBase.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
Modified: cfe/trunk/include/clang/AST/Decl.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=239371&r1=239370&r2=239371&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Mon Jun 8 19:35:49 2015
@@ -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 {
Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=239371&r1=239370&r2=239371&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Mon Jun 8 19:35:49 2015
@@ -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<Module *const *>(this)[-1];
}
void setLocalOwningModule(Module *M) {
- assert(!isFromASTFile() && Hidden &&
+ assert(!isFromASTFile() && Hidden && hasLocalOwningModuleStorage() &&
"should not have a cached owning module");
reinterpret_cast<Module **>(this)[-1] = M;
}
Modified: cfe/trunk/include/clang/Sema/Lookup.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Lookup.h?rev=239371&r1=239370&r2=239371&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Lookup.h (original)
+++ cfe/trunk/include/clang/Sema/Lookup.h Mon Jun 8 19:35:49 2015
@@ -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:
Modified: cfe/trunk/lib/AST/DeclBase.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=239371&r1=239370&r2=239371&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclBase.cpp (original)
+++ cfe/trunk/lib/AST/DeclBase.cpp Mon Jun 8 19:35:49 2015
@@ -80,6 +80,10 @@ Module *Decl::getOwningModuleSlow() cons
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!");
Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=239371&r1=239370&r2=239371&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Mon Jun 8 19:35:49 2015
@@ -1180,6 +1180,16 @@ Module *Sema::getOwningModule(Decl *Enti
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<NamedDecl>(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 *Enti
}
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.
Added: cfe/trunk/test/Modules/Inputs/template-default-args/a.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/template-default-args/a.h?rev=239371&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/template-default-args/a.h (added)
+++ cfe/trunk/test/Modules/Inputs/template-default-args/a.h Mon Jun 8 19:35:49 2015
@@ -0,0 +1,4 @@
+template<typename T = int> struct A {};
+template<typename T> struct B {};
+template<typename T> struct C;
+template<typename T> struct D;
Added: cfe/trunk/test/Modules/Inputs/template-default-args/b.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/template-default-args/b.h?rev=239371&view=auto
==============================================================================
(empty)
Added: cfe/trunk/test/Modules/Inputs/template-default-args/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/template-default-args/module.modulemap?rev=239371&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/template-default-args/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/template-default-args/module.modulemap Mon Jun 8 19:35:49 2015
@@ -0,0 +1 @@
+module X { module A { header "a.h" } module B { header "b.h" } }
Added: cfe/trunk/test/Modules/template-default-args.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/template-default-args.cpp?rev=239371&view=auto
==============================================================================
--- cfe/trunk/test/Modules/template-default-args.cpp (added)
+++ cfe/trunk/test/Modules/template-default-args.cpp Mon Jun 8 19:35:49 2015
@@ -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<typename T> struct A;
+template<typename T> struct B;
+template<typename T> struct C;
+template<typename T = int> struct D;
+
+#include "b.h"
+
+template<typename T = int> struct A {};
+template<typename T> struct B {};
+template<typename T = int> struct B;
+template<typename T = int> struct C;
+template<typename T> struct D {};
+
+A<> a;
+B<> b;
+extern C<> c;
+D<> d;
More information about the cfe-commits
mailing list