<div dir="ltr">This was leading to many crashers. Reverted in r303037.</div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, May 13, 2017 at 1:27 AM, Richard Smith via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rsmith<br>
Date: Fri May 12 18:27:00 2017<br>
New Revision: 302965<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=302965&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=302965&view=rev</a><br>
Log:<br>
[modules] When creating a declaration, cache its owning module immediately<br>
rather than waiting until it's queried.<br>
<br>
Currently this is only applied to local submodule visibility mode, as we don't<br>
yet allocate storage for the owning module in non-local-visibility modules<br>
compilations.<br>
<br>
Modified:<br>
cfe/trunk/include/clang/AST/<wbr>Decl.h<br>
cfe/trunk/include/clang/AST/<wbr>DeclBase.h<br>
cfe/trunk/include/clang/Sema/<wbr>Sema.h<br>
cfe/trunk/lib/AST/ASTDumper.<wbr>cpp<br>
cfe/trunk/lib/AST/Decl.cpp<br>
cfe/trunk/lib/AST/DeclBase.cpp<br>
cfe/trunk/lib/Sema/Sema.cpp<br>
cfe/trunk/lib/Sema/SemaDecl.<wbr>cpp<br>
cfe/trunk/lib/Sema/SemaLookup.<wbr>cpp<br>
<br>
Modified: cfe/trunk/include/clang/AST/<wbr>Decl.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=302965&r1=302964&r2=302965&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/include/<wbr>clang/AST/Decl.h?rev=302965&<wbr>r1=302964&r2=302965&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/include/clang/AST/<wbr>Decl.h (original)<br>
+++ cfe/trunk/include/clang/AST/<wbr>Decl.h Fri May 12 18:27:00 2017<br>
@@ -301,16 +301,6 @@ public:<br>
using Decl::isModulePrivate;<br>
using Decl::setModulePrivate;<br>
<br>
- /// \brief Determine whether this declaration is hidden from name lookup.<br>
- bool isHidden() const { return Hidden; }<br>
-<br>
- /// \brief Set whether this declaration is hidden from name lookup.<br>
- void setHidden(bool Hide) {<br>
- assert((!Hide || isFromASTFile() || hasLocalOwningModuleStorage()) &&<br>
- "declaration with no owning module can't be hidden");<br>
- Hidden = Hide;<br>
- }<br>
-<br>
/// \brief Determine whether this declaration is a C++ class member.<br>
bool isCXXClassMember() const {<br>
const DeclContext *DC = getDeclContext();<br>
<br>
Modified: cfe/trunk/include/clang/AST/<wbr>DeclBase.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=302965&r1=302964&r2=302965&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/include/<wbr>clang/AST/DeclBase.h?rev=<wbr>302965&r1=302964&r2=302965&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/include/clang/AST/<wbr>DeclBase.h (original)<br>
+++ cfe/trunk/include/clang/AST/<wbr>DeclBase.h Fri May 12 18:27:00 2017<br>
@@ -706,6 +706,20 @@ public:<br>
reinterpret_cast<Module **>(this)[-1] = M;<br>
}<br>
<br>
+ Module *getOwningModule() const {<br>
+ return isFromASTFile() ? getImportedOwningModule() : getLocalOwningModule();<br>
+ }<br>
+<br>
+ /// \brief Determine whether this declaration is hidden from name lookup.<br>
+ bool isHidden() const { return Hidden; }<br>
+<br>
+ /// \brief Set whether this declaration is hidden from name lookup.<br>
+ void setHidden(bool Hide) {<br>
+ assert((!Hide || isFromASTFile() || hasLocalOwningModuleStorage()) &&<br>
+ "declaration with no owning module can't be hidden");<br>
+ Hidden = Hide;<br>
+ }<br>
+<br>
unsigned getIdentifierNamespace() const {<br>
return IdentifierNamespace;<br>
}<br>
<br>
Modified: cfe/trunk/include/clang/Sema/<wbr>Sema.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=302965&r1=302964&r2=302965&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/include/<wbr>clang/Sema/Sema.h?rev=302965&<wbr>r1=302964&r2=302965&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/include/clang/Sema/<wbr>Sema.h (original)<br>
+++ cfe/trunk/include/clang/Sema/<wbr>Sema.h Fri May 12 18:27:00 2017<br>
@@ -1463,11 +1463,9 @@ private:<br>
<br>
VisibleModuleSet VisibleModules;<br>
<br>
- Module *CachedFakeTopLevelModule;<br>
-<br>
public:<br>
/// \brief Get the module owning an entity.<br>
- Module *getOwningModule(Decl *Entity);<br>
+ Module *getOwningModule(Decl *Entity) { return Entity->getOwningModule(); }<br>
<br>
/// \brief Make a merged definition of an existing hidden definition \p ND<br>
/// visible at the specified location.<br>
<br>
Modified: cfe/trunk/lib/AST/ASTDumper.<wbr>cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=302965&r1=302964&r2=302965&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/AST/<wbr>ASTDumper.cpp?rev=302965&r1=<wbr>302964&r2=302965&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/AST/ASTDumper.<wbr>cpp (original)<br>
+++ cfe/trunk/lib/AST/ASTDumper.<wbr>cpp Fri May 12 18:27:00 2017<br>
@@ -1038,10 +1038,10 @@ void ASTDumper::dumpDecl(const Decl *D)<br>
dumpSourceRange(D-><wbr>getSourceRange());<br>
OS << ' ';<br>
dumpLocation(D->getLocation())<wbr>;<br>
- if (Module *M = D->getImportedOwningModule())<br>
+ if (D->isFromASTFile())<br>
+ OS << " imported";<br>
+ if (Module *M = D->getOwningModule())<br>
OS << " in " << M->getFullModuleName();<br>
- else if (Module *M = D->getLocalOwningModule())<br>
- OS << " in (local) " << M->getFullModuleName();<br>
if (auto *ND = dyn_cast<NamedDecl>(D))<br>
for (Module *M : D->getASTContext().<wbr>getModulesWithMergedDefinition<wbr>(<br>
const_cast<NamedDecl *>(ND)))<br>
<br>
Modified: cfe/trunk/lib/AST/Decl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=302965&r1=302964&r2=302965&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/AST/<wbr>Decl.cpp?rev=302965&r1=302964&<wbr>r2=302965&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/AST/Decl.cpp (original)<br>
+++ cfe/trunk/lib/AST/Decl.cpp Fri May 12 18:27:00 2017<br>
@@ -47,9 +47,7 @@ bool Decl::isOutOfLine() const {<br>
<br>
TranslationUnitDecl::<wbr>TranslationUnitDecl(ASTContext &ctx)<br>
: Decl(TranslationUnit, nullptr, SourceLocation()),<br>
- DeclContext(TranslationUnit), Ctx(ctx), AnonymousNamespace(nullptr) {<br>
- Hidden = Ctx.getLangOpts().<wbr>ModulesLocalVisibility;<br>
-}<br>
+ DeclContext(TranslationUnit), Ctx(ctx), AnonymousNamespace(nullptr) {}<br>
<br>
//===-------------------------<wbr>------------------------------<wbr>---------------===//<br>
// NamedDecl Implementation<br>
<br>
Modified: cfe/trunk/lib/AST/DeclBase.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=302965&r1=302964&r2=302965&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/AST/<wbr>DeclBase.cpp?rev=302965&r1=<wbr>302964&r2=302965&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/AST/DeclBase.cpp (original)<br>
+++ cfe/trunk/lib/AST/DeclBase.cpp Fri May 12 18:27:00 2017<br>
@@ -83,7 +83,9 @@ void *Decl::operator new(std::size_t Siz<br>
char *Buffer = reinterpret_cast<char *>(<br>
::operator new(ExtraAlign + sizeof(Module *) + Size + Extra, Ctx));<br>
Buffer += ExtraAlign;<br>
- return new (Buffer) Module*(nullptr) + 1;<br>
+ auto *ParentModule =<br>
+ Parent ? cast<Decl>(Parent)-><wbr>getOwningModule() : nullptr;<br>
+ return new (Buffer) Module*(ParentModule) + 1;<br>
}<br>
return ::operator new(Size + Extra, Ctx);<br>
}<br>
@@ -273,6 +275,11 @@ void Decl::setLexicalDeclContext(<wbr>DeclCon<br>
getMultipleDC()->LexicalDC = DC;<br>
}<br>
Hidden = cast<Decl>(DC)->Hidden;<br>
+ if (Hidden && !isFromASTFile()) {<br>
+ assert(<wbr>hasLocalOwningModuleStorage() &&<br>
+ "hidden local declaration without local submodule visibility?");<br>
+ setLocalOwningModule(cast<<wbr>Decl>(DC)->getOwningModule());<br>
+ }<br>
}<br>
<br>
void Decl::setDeclContextsImpl(<wbr>DeclContext *SemaDC, DeclContext *LexicalDC,<br>
<br>
Modified: cfe/trunk/lib/Sema/Sema.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=302965&r1=302964&r2=302965&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/Sema/<wbr>Sema.cpp?rev=302965&r1=302964&<wbr>r2=302965&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Sema/Sema.cpp (original)<br>
+++ cfe/trunk/lib/Sema/Sema.cpp Fri May 12 18:27:00 2017<br>
@@ -93,11 +93,10 @@ Sema::Sema(Preprocessor &pp, ASTContext<br>
ValueWithBytesObjCTypeMethod(<wbr>nullptr), NSArrayDecl(nullptr),<br>
ArrayWithObjectsMethod(<wbr>nullptr), NSDictionaryDecl(nullptr),<br>
DictionaryWithObjectsMethod(<wbr>nullptr), GlobalNewDeleteDeclared(false)<wbr>,<br>
- TUKind(TUKind), NumSFINAEErrors(0), CachedFakeTopLevelModule(<wbr>nullptr),<br>
- AccessCheckingSFINAE(false), InNonInstantiationSFINAEContex<wbr>t(false),<br>
- NonInstantiationEntries(0), ArgumentPackSubstitutionIndex(<wbr>-1),<br>
- CurrentInstantiationScope(<wbr>nullptr), DisableTypoCorrection(false),<br>
- TyposCorrected(0), AnalysisWarnings(*this),<br>
+ TUKind(TUKind), NumSFINAEErrors(0), AccessCheckingSFINAE(false),<br>
+ InNonInstantiationSFINAEContex<wbr>t(false), NonInstantiationEntries(0),<br>
+ ArgumentPackSubstitutionIndex(<wbr>-1), CurrentInstantiationScope(<wbr>nullptr),<br>
+ DisableTypoCorrection(false), TyposCorrected(0), AnalysisWarnings(*this),<br>
ThreadSafetyDeclCache(nullptr)<wbr>, VarDataSharingAttributesStack(<wbr>nullptr),<br>
CurScope(nullptr), Ident_super(nullptr), Ident___float128(nullptr) {<br>
TUScope = nullptr;<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDecl.<wbr>cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=302965&r1=302964&r2=302965&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/Sema/<wbr>SemaDecl.cpp?rev=302965&r1=<wbr>302964&r2=302965&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Sema/SemaDecl.<wbr>cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDecl.<wbr>cpp Fri May 12 18:27:00 2017<br>
@@ -16034,6 +16034,14 @@ void Sema::ActOnModuleBegin(<wbr>SourceLocati<br>
ModuleScopes.back().<wbr>OuterVisibleModules = std::move(VisibleModules);<br>
<br>
VisibleModules.setVisible(Mod, DirectiveLoc);<br>
+<br>
+ // The enclosing context is now part of this module.<br>
+ // FIXME: Consider creating a child DeclContext to hold the entities<br>
+ // lexically within the module.<br>
+ if (getLangOpts().<wbr>ModulesLocalVisibility) {<br>
+ cast<Decl>(CurContext)-><wbr>setHidden(true);<br>
+ cast<Decl>(CurContext)-><wbr>setLocalOwningModule(Mod);<br>
+ }<br>
}<br>
<br>
void Sema::ActOnModuleEnd(<wbr>SourceLocation EomLoc, Module *Mod) {<br>
@@ -16062,6 +16070,13 @@ void Sema::ActOnModuleEnd(<wbr>SourceLocation<br>
DirectiveLoc = EomLoc;<br>
}<br>
BuildModuleInclude(<wbr>DirectiveLoc, Mod);<br>
+<br>
+ // Any further declarations are in whatever module we returned to.<br>
+ if (getLangOpts().<wbr>ModulesLocalVisibility) {<br>
+ cast<Decl>(CurContext)-><wbr>setLocalOwningModule(<wbr>getCurrentModule());<br>
+ if (!getCurrentModule())<br>
+ cast<Decl>(CurContext)-><wbr>setHidden(false);<br>
+ }<br>
}<br>
<br>
void Sema::<wbr>createImplicitModuleImportForE<wbr>rrorRecovery(SourceLocation Loc,<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaLookup.<wbr>cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=302965&r1=302964&r2=302965&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/Sema/<wbr>SemaLookup.cpp?rev=302965&r1=<wbr>302964&r2=302965&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Sema/SemaLookup.<wbr>cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaLookup.<wbr>cpp Fri May 12 18:27:00 2017<br>
@@ -1326,62 +1326,6 @@ bool Sema::CppLookupName(<wbr>LookupResult &R<br>
return !R.empty();<br>
}<br>
<br>
-Module *Sema::getOwningModule(Decl *Entity) {<br>
- // If it's imported, grab its owning module.<br>
- Module *M = Entity-><wbr>getImportedOwningModule();<br>
- if (M || !isa<NamedDecl>(Entity) || !cast<NamedDecl>(Entity)-><wbr>isHidden())<br>
- return M;<br>
- assert(!Entity->isFromASTFile(<wbr>) &&<br>
- "hidden entity from AST file has no owning module");<br>
-<br>
- if (!getLangOpts().<wbr>ModulesLocalVisibility) {<br>
- // If we're not tracking visibility locally, the only way a declaration<br>
- // can be hidden and local is if it's hidden because it's parent is (for<br>
- // instance, maybe this is a lazily-declared special member of an imported<br>
- // class).<br>
- auto *Parent = cast<NamedDecl>(Entity-><wbr>getDeclContext());<br>
- assert(Parent->isHidden() && "unexpectedly hidden decl");<br>
- return getOwningModule(Parent);<br>
- }<br>
-<br>
- // It's local and hidden; grab or compute its owning module.<br>
- M = Entity->getLocalOwningModule()<wbr>;<br>
- if (M)<br>
- return M;<br>
-<br>
- if (auto *Containing =<br>
- PP.<wbr>getModuleContainingLocation(<wbr>Entity->getLocation())) {<br>
- M = Containing;<br>
- } else if (Entity->isInvalidDecl() || Entity->getLocation().<wbr>isInvalid()) {<br>
- // Don't bother tracking visibility for invalid declarations with broken<br>
- // locations.<br>
- cast<NamedDecl>(Entity)-><wbr>setHidden(false);<br>
- } else {<br>
- // We need to assign a module to an entity that exists outside of any<br>
- // module, so that we can hide it from modules that we textually enter.<br>
- // Invent a fake module for all such entities.<br>
- if (!CachedFakeTopLevelModule) {<br>
- CachedFakeTopLevelModule =<br>
- PP.getHeaderSearchInfo().<wbr>getModuleMap().<wbr>findOrCreateModule(<br>
- "<top-level>", nullptr, false, false).first;<br>
-<br>
- auto &SrcMgr = PP.getSourceManager();<br>
- SourceLocation StartLoc =<br>
- SrcMgr.getLocForStartOfFile(<wbr>SrcMgr.getMainFileID());<br>
- auto &TopLevel = ModuleScopes.empty()<br>
- ? VisibleModules<br>
- : ModuleScopes[0].<wbr>OuterVisibleModules;<br>
- TopLevel.setVisible(<wbr>CachedFakeTopLevelModule, StartLoc);<br>
- }<br>
-<br>
- M = CachedFakeTopLevelModule;<br>
- }<br>
-<br>
- if (M)<br>
- Entity->setLocalOwningModule(<wbr>M);<br>
- return M;<br>
-}<br>
-<br>
void Sema::<wbr>makeMergedDefinitionVisible(<wbr>NamedDecl *ND) {<br>
if (auto *M = getCurrentModule())<br>
Context.<wbr>mergeDefinitionIntoModule(ND, M);<br>
@@ -1520,7 +1464,6 @@ bool LookupResult::isVisibleSlow(<wbr>Sema &S<br>
if (SemaRef.getLangOpts().<wbr>ModulesLocalVisibility) {<br>
DeclModule = SemaRef.getOwningModule(D);<br>
if (!DeclModule) {<br>
- // getOwningModule() may have decided the declaration should not be hidden.<br>
assert(!D->isHidden() && "hidden decl not from a module");<br>
return true;<br>
}<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>