vsapsai updated this revision to Diff 389379.
vsapsai added a comment.
Mark identifiers as out-of-date not only on loading a new module but on making
a (sub)module visible too.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114411/new/
https://reviews.llvm.org/D114411
Files:
clang/include/clang/Serialization/ASTReader.h
clang/lib/Lex/Preprocessor.cpp
clang/lib/Sema/SemaModule.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/test/Modules/ambiguous-enum-lookup.m
Index: clang/test/Modules/ambiguous-enum-lookup.m
===================================================================
--- /dev/null
+++ clang/test/Modules/ambiguous-enum-lookup.m
@@ -0,0 +1,42 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -F %t/Frameworks %t/test.m
+
+//--- Frameworks/NonModular.framework/Headers/NonModular.h
+#ifndef NonModular_NonModular_h
+#define NonModular_NonModular_h
+struct SomeStruct {
+ int x;
+};
+
+enum SomeEnum {
+ kEnumValue = 1,
+};
+#endif
+
+//--- Frameworks/PiecewiseVisible.framework/Headers/InitiallyVisible.h
+// empty
+
+//--- Frameworks/PiecewiseVisible.framework/Headers/InitiallyHidden.h
+#include <NonModular/NonModular.h>
+
+//--- Frameworks/PiecewiseVisible.framework/Modules/module.modulemap
+framework module PiecewiseVisible {
+ header "InitiallyVisible.h"
+ export *
+
+ explicit module HiddenPiece {
+ header "InitiallyHidden.h"
+ export *
+ }
+}
+
+//--- test.m
+#include <PiecewiseVisible/InitiallyVisible.h>
+#include <NonModular/NonModular.h>
+#include <PiecewiseVisible/InitiallyHidden.h>
+
+void test() {
+ struct SomeStruct s;
+ s.x = kEnumValue;
+}
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===================================================================
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -269,6 +269,7 @@
}
void ASTDeclWriter::Visit(Decl *D) {
+ Record.push_back(Writer.getSubmoduleID(D->getOwningModule()));
DeclVisitor<ASTDeclWriter>::Visit(D);
// Source locations require array (variable-length) abbreviations. The
@@ -1911,6 +1912,7 @@
// Abbreviation for DECL_FIELD
Abv = std::make_shared<BitCodeAbbrev>();
Abv->Add(BitCodeAbbrevOp(serialization::DECL_FIELD));
+ Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // SubmoduleID
// Decl
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // DeclContext
Abv->Add(BitCodeAbbrevOp(0)); // LexicalDeclContext
@@ -1944,6 +1946,7 @@
// Abbreviation for DECL_OBJC_IVAR
Abv = std::make_shared<BitCodeAbbrev>();
Abv->Add(BitCodeAbbrevOp(serialization::DECL_OBJC_IVAR));
+ Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // SubmoduleID
// Decl
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // DeclContext
Abv->Add(BitCodeAbbrevOp(0)); // LexicalDeclContext
@@ -1980,6 +1983,7 @@
// Abbreviation for DECL_ENUM
Abv = std::make_shared<BitCodeAbbrev>();
Abv->Add(BitCodeAbbrevOp(serialization::DECL_ENUM));
+ Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // SubmoduleID
// Redeclarable
Abv->Add(BitCodeAbbrevOp(0)); // No redeclaration
// Decl
@@ -2030,6 +2034,7 @@
// Abbreviation for DECL_RECORD
Abv = std::make_shared<BitCodeAbbrev>();
Abv->Add(BitCodeAbbrevOp(serialization::DECL_RECORD));
+ Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // SubmoduleID
// Redeclarable
Abv->Add(BitCodeAbbrevOp(0)); // No redeclaration
// Decl
@@ -2092,6 +2097,7 @@
// Abbreviation for DECL_PARM_VAR
Abv = std::make_shared<BitCodeAbbrev>();
Abv->Add(BitCodeAbbrevOp(serialization::DECL_PARM_VAR));
+ Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // SubmoduleID
// Redeclarable
Abv->Add(BitCodeAbbrevOp(0)); // No redeclaration
// Decl
@@ -2140,6 +2146,7 @@
// Abbreviation for DECL_TYPEDEF
Abv = std::make_shared<BitCodeAbbrev>();
Abv->Add(BitCodeAbbrevOp(serialization::DECL_TYPEDEF));
+ Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // SubmoduleID
// Redeclarable
Abv->Add(BitCodeAbbrevOp(0)); // No redeclaration
// Decl
@@ -2169,6 +2176,7 @@
// Abbreviation for DECL_VAR
Abv = std::make_shared<BitCodeAbbrev>();
Abv->Add(BitCodeAbbrevOp(serialization::DECL_VAR));
+ Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // SubmoduleID
// Redeclarable
Abv->Add(BitCodeAbbrevOp(0)); // No redeclaration
// Decl
@@ -2221,6 +2229,7 @@
// Abbreviation for DECL_CXX_METHOD
Abv = std::make_shared<BitCodeAbbrev>();
Abv->Add(BitCodeAbbrevOp(serialization::DECL_CXX_METHOD));
+ Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // SubmoduleID
// RedeclarableDecl
Abv->Add(BitCodeAbbrevOp(0)); // CanonicalDecl
// Decl
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -3870,6 +3870,15 @@
llvm::report_fatal_error(
Twine("ASTReader::readDeclRecord failed reading decl code: ") +
toString(MaybeDeclCode.takeError()));
+ if (unsigned DeclSubmoduleID = getGlobalSubmoduleID(*Loc.F, Record.readInt())) {
+ if (!HackyIsDeserializingSpecialTypes && SemaObj) {
+ Module *DeclOwner = getSubmodule(DeclSubmoduleID);
+ if (DeclOwner && !SemaObj->isModuleVisible(DeclOwner)) {
+ // Skip deserialization of Decls from a hidden module.
+ return nullptr;
+ }
+ }
+ }
switch ((DeclCode)MaybeDeclCode.get()) {
case DECL_CONTEXT_LEXICAL:
case DECL_CONTEXT_VISIBLE:
@@ -4356,9 +4365,11 @@
// we should instead generate one loop per kind and dispatch up-front?
Decl *MostRecent = FirstLocal;
for (unsigned I = 0, N = Record.size(); I != N; ++I) {
- auto *D = GetLocalDecl(*M, Record[N - I - 1]);
- ASTDeclReader::attachPreviousDecl(*this, D, MostRecent, CanonDecl);
- MostRecent = D;
+ // A local decl is from the same module but can be from a hidden submodule.
+ if (Decl *D = GetLocalDecl(*M, Record[N - I - 1])) {
+ ASTDeclReader::attachPreviousDecl(*this, D, MostRecent, CanonDecl);
+ MostRecent = D;
+ }
}
ASTDeclReader::attachLatestDecl(CanonDecl, MostRecent);
}
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -4862,6 +4862,7 @@
// built-in types. Right now, we just ignore the problem.
// Load the special types.
+ HackyIsDeserializingSpecialTypes = true;
if (SpecialTypes.size() >= NumSpecialTypeIDs) {
if (unsigned String = SpecialTypes[SPECIAL_TYPE_CF_CONSTANT_STRING]) {
if (!Context.CFConstantStringTypeDecl)
@@ -4964,6 +4965,7 @@
}
}
}
+ HackyIsDeserializingSpecialTypes = false;
ReadPragmaDiagnosticMappings(Context.getDiagnostics());
@@ -7628,9 +7630,9 @@
SmallVector<NamedDecl *, 64> Decls;
llvm::SmallPtrSet<NamedDecl *, 8> Found;
for (DeclID ID : It->second.Table.find(Name)) {
- NamedDecl *ND = cast<NamedDecl>(GetDecl(ID));
- if (ND->getDeclName() == Name && Found.insert(ND).second)
- Decls.push_back(ND);
+ if (NamedDecl *ND = cast_or_null<NamedDecl>(GetDecl(ID)))
+ if (ND->getDeclName() == Name && Found.insert(ND).second)
+ Decls.push_back(ND);
}
++NumVisibleDeclContextsRead;
@@ -8474,18 +8476,20 @@
continue;
}
- NamedDecl *D = cast<NamedDecl>(GetDecl(DeclIDs[I]));
+ if (Decl *DeserializedD = GetDecl(DeclIDs[I])) {
+ NamedDecl *D = cast<NamedDecl>(DeserializedD);
- // If we're simply supposed to record the declarations, do so now.
- if (Decls) {
- Decls->push_back(D);
- continue;
- }
+ // If we're simply supposed to record the declarations, do so now.
+ if (Decls) {
+ Decls->push_back(D);
+ continue;
+ }
- // Introduce this declaration into the translation-unit scope
- // and add it to the declaration chain for this identifier, so
- // that (unqualified) name lookup will find it.
- pushExternalDeclIntoScope(D, II);
+ // Introduce this declaration into the translation-unit scope
+ // and add it to the declaration chain for this identifier, so
+ // that (unqualified) name lookup will find it.
+ pushExternalDeclIntoScope(D, II);
+ }
}
}
Index: clang/lib/Sema/SemaModule.cpp
===================================================================
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -407,6 +407,10 @@
}
void Sema::BuildModuleInclude(SourceLocation DirectiveLoc, Module *Mod) {
+ // Make the module visible before adding a module initializer.
+ getModuleLoader().makeModuleVisible(Mod, Module::AllVisible, DirectiveLoc);
+ VisibleModules.setVisible(Mod, DirectiveLoc);
+
// Determine whether we're in the #include buffer for a module. The #includes
// in that buffer do not qualify as module imports; they're just an
// implementation detail of us building the module.
@@ -430,9 +434,6 @@
TU->addDecl(ImportD);
Consumer.HandleImplicitImportDecl(ImportD);
}
-
- getModuleLoader().makeModuleVisible(Mod, Module::AllVisible, DirectiveLoc);
- VisibleModules.setVisible(Mod, DirectiveLoc);
}
void Sema::ActOnModuleBegin(SourceLocation DirectiveLoc, Module *Mod) {
Index: clang/lib/Lex/Preprocessor.cpp
===================================================================
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -1304,8 +1304,9 @@
}
void Preprocessor::makeModuleVisible(Module *M, SourceLocation Loc) {
+ bool HasAddedMoreModules = false;
CurSubmoduleState->VisibleModules.setVisible(
- M, Loc, [](Module *) {},
+ M, Loc, [&HasAddedMoreModules](Module *) { HasAddedMoreModules = true; },
[&](ArrayRef<Module *> Path, Module *Conflict, StringRef Message) {
// FIXME: Include the path in the diagnostic.
// FIXME: Include the import location for the conflicting module.
@@ -1318,6 +1319,18 @@
// Add this module to the imports list of the currently-built submodule.
if (!BuildingSubmoduleStack.empty() && M != BuildingSubmoduleStack.back().M)
BuildingSubmoduleStack.back().M->Imports.insert(M);
+
+ // EXPERIMENTAL: Setting a (sub)module visible allows to deserialize its
+ // decls. For this reason marking the identifiers out-of-date like
+ // in `ASTReader::ReadAST`.
+ if (HasAddedMoreModules && getExternalSource()) {
+ if (!getLangOpts().CPlusPlus) {
+ for (IdentifierTable::iterator Id = getIdentifierTable().begin(),
+ IdEnd = getIdentifierTable().end();
+ Id != IdEnd; ++Id)
+ Id->second->setOutOfDate(true);
+ }
+ }
}
bool Preprocessor::FinishLexStringLiteral(Token &Result, std::string &String,
Index: clang/include/clang/Serialization/ASTReader.h
===================================================================
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -406,6 +406,11 @@
bool OwnsDeserializationListener = false;
+ // Special types are deserialized before we have a chance to mark a submodule
+ // as visible or hidden. Add a hacky way to allow deserialization of types'
+ // decls even if we don't know if a corresponding module is visible.
+ bool HackyIsDeserializingSpecialTypes = false;
+
SourceManager &SourceMgr;
FileManager &FileMgr;
const PCHContainerReader &PCHContainerRdr;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
