jansvoboda11 created this revision.
jansvoboda11 added reviewers: dexonsmith, Bigcheese, vsapsai, ilyakuteev.
Herald added subscribers: ributzka, mgrang.
Herald added a project: All.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
In D106876 <https://reviews.llvm.org/D106876>, we stopped serializing module
map files that didn't affect compilation of the current module.
However, since each `SourceLocation` is simply an offset into `SourceManager`'s
global buffer of concatenated input files in, these need to be adjusted during
serialization. Otherwise, they can incorrectly point after the buffer or into
subsequent input file.
This patch starts adjusting `SourceLocation`s, `FileID`s and other
`SourceManager` offsets in `ASTWriter`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D136624
Files:
clang/include/clang/Basic/SourceManager.h
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/test/Modules/non-affecting-module-maps-source-locations.m
Index: clang/test/Modules/non-affecting-module-maps-source-locations.m
===================================================================
--- /dev/null
+++ clang/test/Modules/non-affecting-module-maps-source-locations.m
@@ -0,0 +1,32 @@
+// This patch tests that non-affecting files are serialized in such way that
+// does not break subsequent source locations (e.g. in serialized pragma
+// diagnostic mappings).
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- tu.m
+#include "first.h"
+
+//--- first/module.modulemap
+module first { header "first.h" }
+//--- first/first.h
+@import second;
+#pragma clang diagnostic ignored "-Wunreachable-code"
+
+//--- second/extra/module.modulemap
+module second_extra {}
+//--- second/module.modulemap
+module second { module sub { header "second_sub.h" } }
+//--- second/second_sub.h
+@import third;
+
+//--- third/module.modulemap
+module third { header "third.h" }
+//--- third/third.h
+#if __has_feature(nullability)
+#endif
+#if __has_feature(nullability)
+#endif
+
+// RUN: %clang_cc1 -I %t/first -I %t/second -I %t/third -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache %t/tu.m -o %t/tu.o
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===================================================================
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2457,11 +2457,12 @@
SourceLocation Loc = D->getLocation();
unsigned Index = ID - FirstDeclID;
if (DeclOffsets.size() == Index)
- DeclOffsets.emplace_back(Loc, Offset, DeclTypesBlockStartOffset);
+ DeclOffsets.emplace_back(getAdjustedLocation(Loc), Offset,
+ DeclTypesBlockStartOffset);
else if (DeclOffsets.size() < Index) {
// FIXME: Can/should this happen?
DeclOffsets.resize(Index+1);
- DeclOffsets[Index].setLocation(Loc);
+ DeclOffsets[Index].setLocation(getAdjustedLocation(Loc));
DeclOffsets[Index].setBitOffset(Offset, DeclTypesBlockStartOffset);
} else {
llvm_unreachable("declarations should be emitted in ID order");
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -161,8 +161,8 @@
namespace {
-std::set<const FileEntry *> GetAllModuleMaps(const HeaderSearch &HS,
- Module *RootModule) {
+std::set<const FileEntry *> GetAffectingModuleMaps(const HeaderSearch &HS,
+ Module *RootModule) {
std::set<const FileEntry *> ModuleMaps{};
std::set<const Module *> ProcessedModules;
SmallVector<const Module *> ModulesToProcess{RootModule};
@@ -1468,23 +1468,16 @@
Record.clear();
Record.push_back(ORIGINAL_FILE);
- Record.push_back(SM.getMainFileID().getOpaqueValue());
+ AddFileID(SM.getMainFileID(), Record);
EmitRecordWithPath(FileAbbrevCode, Record, MainFile->getName());
}
Record.clear();
- Record.push_back(SM.getMainFileID().getOpaqueValue());
+ AddFileID(SM.getMainFileID(), Record);
Stream.EmitRecord(ORIGINAL_FILE_ID, Record);
- std::set<const FileEntry *> AffectingModuleMaps;
- if (WritingModule) {
- AffectingModuleMaps =
- GetAllModuleMaps(PP.getHeaderSearchInfo(), WritingModule);
- }
-
WriteInputFiles(Context.SourceMgr,
- PP.getHeaderSearchInfo().getHeaderSearchOpts(),
- AffectingModuleMaps);
+ PP.getHeaderSearchInfo().getHeaderSearchOpts());
Stream.ExitBlock();
}
@@ -1502,9 +1495,8 @@
} // namespace
-void ASTWriter::WriteInputFiles(
- SourceManager &SourceMgr, HeaderSearchOptions &HSOpts,
- std::set<const FileEntry *> &AffectingModuleMaps) {
+void ASTWriter::WriteInputFiles(SourceManager &SourceMgr,
+ HeaderSearchOptions &HSOpts) {
using namespace llvm;
Stream.EnterSubblock(INPUT_FILES_BLOCK_ID, 4);
@@ -1544,15 +1536,12 @@
if (!Cache->OrigEntry)
continue;
+ // Do not emit inputs that do not affect current module.
if (isModuleMap(File.getFileCharacteristic()) &&
!isSystem(File.getFileCharacteristic()) &&
- !AffectingModuleMaps.empty() &&
- AffectingModuleMaps.find(Cache->OrigEntry) ==
- AffectingModuleMaps.end()) {
- SkippedModuleMaps.insert(Cache->OrigEntry);
- // Do not emit modulemaps that do not affect current module.
+ std::binary_search(NonAffectingInputs.begin(), NonAffectingInputs.end(),
+ FileID::get(I)))
continue;
- }
InputFileEntry Entry;
Entry.File = Cache->OrigEntry;
@@ -2062,14 +2051,14 @@
Record.push_back(Code);
// Starting offset of this entry within this module, so skip the dummy.
- Record.push_back(SLoc->getOffset() - 2);
+ Record.push_back(getAdjustedOffset(SLoc->getOffset()) - 2);
if (SLoc->isFile()) {
const SrcMgr::FileInfo &File = SLoc->getFile();
const SrcMgr::ContentCache *Content = &File.getContentCache();
- if (Content->OrigEntry && !SkippedModuleMaps.empty() &&
- SkippedModuleMaps.find(Content->OrigEntry) !=
- SkippedModuleMaps.end()) {
+ if (Content->OrigEntry &&
+ llvm::is_contained(NonAffectingInputs, FID)) {
// Do not emit files that were not listed as inputs.
+ SLocEntryOffsets.pop_back();
continue;
}
AddSourceLocation(File.getIncludeLoc(), Record);
@@ -2145,7 +2134,7 @@
SourceLocation::UIntTy NextOffset = SourceMgr.getNextLocalOffset();
if (I + 1 != N)
NextOffset = SourceMgr.getLocalSLocEntry(I + 1).getOffset();
- Record.push_back(NextOffset - SLoc->getOffset() - 1);
+ Record.push_back(getAdjustedOffset(NextOffset - SLoc->getOffset()) - 1);
Stream.EmitRecordWithAbbrev(SLocExpansionAbbrv, Record);
}
}
@@ -2169,7 +2158,7 @@
{
RecordData::value_type Record[] = {
SOURCE_LOCATION_OFFSETS, SLocEntryOffsets.size(),
- SourceMgr.getNextLocalOffset() - 1 /* skip dummy */,
+ getAdjustedOffset(SourceMgr.getNextLocalOffset()) - 1 /* skip dummy */,
SLocEntryOffsetsBase - SourceManagerBlockOffset};
Stream.EmitRecordWithBlob(SLocOffsetsAbbrev, Record,
bytes(SLocEntryOffsets));
@@ -2569,7 +2558,7 @@
uint64_t Offset = Stream.GetCurrentBitNo() - MacroOffsetsBase;
assert((Offset >> 32) == 0 && "Preprocessed entity offset too large");
PreprocessedEntityOffsets.push_back(
- PPEntityOffset((*E)->getSourceRange(), Offset));
+ PPEntityOffset(getAdjustedRange((*E)->getSourceRange()), Offset));
if (auto *MD = dyn_cast<MacroDefinitionRecord>(*E)) {
// Record this macro definition's ID.
@@ -3010,13 +2999,14 @@
continue;
++NumLocations;
+ // TODO: Only write the FileID.
SourceLocation Loc = Diag.SourceMgr->getComposedLoc(FileIDAndFile.first, 0);
assert(!Loc.isInvalid() && "start loc for valid FileID is invalid");
AddSourceLocation(Loc, Record);
Record.push_back(FileIDAndFile.second.StateTransitions.size());
for (auto &StatePoint : FileIDAndFile.second.StateTransitions) {
- Record.push_back(StatePoint.Offset);
+ Record.push_back(getAdjustedOffset(StatePoint.Offset));
AddDiagState(StatePoint.State, false);
}
}
@@ -4526,6 +4516,52 @@
}
}
+void ASTWriter::collectNonAffectingInputFiles() {
+ if (!WritingModule)
+ return;
+
+ std::set<const FileEntry *> AffectingModuleMaps =
+ GetAffectingModuleMaps(PP->getHeaderSearchInfo(), WritingModule);
+
+ SourceManager &SrcMgr = PP->getSourceManager();
+
+ for (unsigned I = 1, N = SrcMgr.local_sloc_entry_size(); I != N; ++I) {
+ // Get this source location entry.
+ const SrcMgr::SLocEntry *SLoc = &SrcMgr.getLocalSLocEntry(I);
+ assert(&SrcMgr.getSLocEntry(FileID::get(I)) == SLoc);
+
+ if (!SLoc->isFile())
+ continue;
+ const SrcMgr::FileInfo &File = SLoc->getFile();
+ const SrcMgr::ContentCache *Cache = &File.getContentCache();
+ if (!Cache->OrigEntry)
+ continue;
+
+ if (isModuleMap(File.getFileCharacteristic()) &&
+ !isSystem(File.getFileCharacteristic()) &&
+ !AffectingModuleMaps.empty() &&
+ AffectingModuleMaps.find(Cache->OrigEntry) ==
+ AffectingModuleMaps.end()) {
+ NonAffectingInputs.push_back(FileID::get(I));
+ }
+ }
+
+ llvm::sort(NonAffectingInputs);
+
+ unsigned Size = 0;
+ unsigned Count = 0;
+ NonAffectingInputOffsetAdjustments.reserve(NonAffectingInputs.size() + 1);
+ NonAffectingInputFileIDAdjustments.reserve(NonAffectingInputs.size() + 1);
+ NonAffectingInputOffsetAdjustments.push_back(Size);
+ NonAffectingInputFileIDAdjustments.push_back(Count);
+ for (FileID FID : NonAffectingInputs) {
+ Size += SrcMgr.getFileIDSize(FID);
+ Count += 1;
+ NonAffectingInputOffsetAdjustments.push_back(Size);
+ NonAffectingInputFileIDAdjustments.push_back(Count);
+ }
+}
+
ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot,
Module *WritingModule) {
using namespace llvm;
@@ -4539,6 +4575,8 @@
ASTContext &Context = SemaRef.Context;
Preprocessor &PP = SemaRef.PP;
+ collectNonAffectingInputFiles();
+
// Set up predefined declaration IDs.
auto RegisterPredefDecl = [&] (Decl *D, PredefinedDeclIDs ID) {
if (D) {
@@ -5227,8 +5265,52 @@
Record.push_back(Raw);
}
+void ASTWriter::AddFileID(FileID FID, RecordDataImpl &Record) {
+ Record.push_back(getAdjustedFileID(FID).getOpaqueValue());
+}
+
+FileID ASTWriter::getAdjustedFileID(FileID FID) const {
+ if (FID.isInvalid() || PP->getSourceManager().isLoadedFileID(FID) ||
+ NonAffectingInputs.empty())
+ return FID;
+ auto It = llvm::lower_bound(NonAffectingInputs, FID);
+ unsigned Idx = std::distance(NonAffectingInputs.begin(), It);
+ unsigned Offset = NonAffectingInputFileIDAdjustments[Idx];
+ return FileID::get(FID.getOpaqueValue() - Offset);
+}
+
+SourceLocation::UIntTy
+ASTWriter::getAdjustment(SourceLocation::UIntTy Offset) const {
+ if (PP->getSourceManager().isLoadedOffset(Offset) ||
+ NonAffectingInputs.empty())
+ return 0;
+ auto It = (Offset == PP->getSourceManager().getNextLocalOffset())
+ ? NonAffectingInputs.end()
+ : llvm::lower_bound(NonAffectingInputs,
+ PP->getSourceManager().getFileID(Offset));
+ unsigned Idx = std::distance(NonAffectingInputs.begin(), It);
+ return NonAffectingInputOffsetAdjustments[Idx];
+}
+
+SourceLocation::UIntTy
+ASTWriter::getAdjustedOffset(SourceLocation::UIntTy Offset) const {
+ return Offset - getAdjustment(Offset);
+}
+
+SourceLocation ASTWriter::getAdjustedLocation(SourceLocation Loc) const {
+ if (Loc.isInvalid())
+ return Loc;
+ return Loc.getLocWithOffset(-getAdjustment(Loc.getOffset()));
+}
+
+SourceRange ASTWriter::getAdjustedRange(SourceRange Range) const {
+ return SourceRange(getAdjustedLocation(Range.getBegin()),
+ getAdjustedLocation(Range.getEnd()));
+}
+
void ASTWriter::AddSourceLocation(SourceLocation Loc, RecordDataImpl &Record,
SourceLocationSequence *Seq) {
+ Loc = getAdjustedLocation(Loc);
Record.push_back(SourceLocationEncoding::encode(Loc, Seq));
}
Index: clang/include/clang/Serialization/ASTWriter.h
===================================================================
--- clang/include/clang/Serialization/ASTWriter.h
+++ clang/include/clang/Serialization/ASTWriter.h
@@ -444,8 +444,29 @@
std::vector<std::unique_ptr<ModuleFileExtensionWriter>>
ModuleFileExtensionWriters;
- /// User ModuleMaps skipped when writing control block.
- std::set<const FileEntry *> SkippedModuleMaps;
+ /// Sorted input files that did not affect the compilation.
+ std::vector<FileID> NonAffectingInputs;
+ /// Exclusive prefix sum of the lengths of preceding non-affecting inputs.
+ std::vector<SourceLocation::UIntTy> NonAffectingInputOffsetAdjustments;
+ /// Exclusive prefix sum of the count of preceding non-affecting inputs.
+ std::vector<unsigned> NonAffectingInputFileIDAdjustments;
+
+ /// Collects input files that didn't affect compilation of the current module,
+ /// and initializes data structures necessary for leaving those files out
+ /// during \c SourceManager serialization.
+ void collectNonAffectingInputFiles();
+
+ /// Prepares the specified \c FileID for serialization, accounting for any
+ /// non-affecting files.
+ FileID getAdjustedFileID(FileID FID) const;
+ /// Prepares the specified \c SourceLocation for serialization, accounting for
+ /// any non-affecting files.
+ SourceLocation getAdjustedLocation(SourceLocation Loc) const;
+ /// Prepares the specified \c SourceRange for serialization, accounting for
+ /// any non-affecting files.
+ SourceRange getAdjustedRange(SourceRange Range) const;
+ SourceLocation::UIntTy getAdjustedOffset(SourceLocation::UIntTy Offset) const;
+ SourceLocation::UIntTy getAdjustment(SourceLocation::UIntTy Offset) const;
/// Retrieve or create a submodule ID for this module.
unsigned getSubmoduleID(Module *Mod);
@@ -465,8 +486,7 @@
static std::pair<ASTFileSignature, ASTFileSignature>
createSignature(StringRef AllBytes, StringRef ASTBlockBytes);
- void WriteInputFiles(SourceManager &SourceMgr, HeaderSearchOptions &HSOpts,
- std::set<const FileEntry *> &AffectingModuleMaps);
+ void WriteInputFiles(SourceManager &SourceMgr, HeaderSearchOptions &HSOpts);
void WriteSourceManagerBlock(SourceManager &SourceMgr,
const Preprocessor &PP);
void writeIncludedFiles(raw_ostream &Out, const Preprocessor &PP);
@@ -582,6 +602,9 @@
void AddAlignPackInfo(const Sema::AlignPackInfo &Info,
RecordDataImpl &Record);
+ /// Emit a FileID.
+ void AddFileID(FileID, RecordDataImpl &Record);
+
/// Emit a source location.
void AddSourceLocation(SourceLocation Loc, RecordDataImpl &Record,
LocSeq *Seq = nullptr);
Index: clang/include/clang/Basic/SourceManager.h
===================================================================
--- clang/include/clang/Basic/SourceManager.h
+++ clang/include/clang/Basic/SourceManager.h
@@ -1114,13 +1114,7 @@
/// the entry in SLocEntryTable which contains the specified location.
///
FileID getFileID(SourceLocation SpellingLoc) const {
- SourceLocation::UIntTy SLocOffset = SpellingLoc.getOffset();
-
- // If our one-entry cache covers this offset, just return it.
- if (isOffsetInFileID(LastFileIDLookup, SLocOffset))
- return LastFileIDLookup;
-
- return getFileIDSlow(SLocOffset);
+ return getFileID(SpellingLoc.getOffset());
}
/// Return the filename of the file containing a SourceLocation.
@@ -1747,12 +1741,12 @@
/// Returns true if \p Loc came from a PCH/Module.
bool isLoadedSourceLocation(SourceLocation Loc) const {
- return Loc.getOffset() >= CurrentLoadedOffset;
+ return isLoadedOffset(Loc.getOffset());
}
/// Returns true if \p Loc did not come from a PCH/Module.
bool isLocalSourceLocation(SourceLocation Loc) const {
- return Loc.getOffset() < NextLocalOffset;
+ return isLocalOffset(Loc.getOffset());
}
/// Returns true if \p FID came from a PCH/Module.
@@ -1822,6 +1816,22 @@
return getLoadedSLocEntry(static_cast<unsigned>(-ID - 2), Invalid);
}
+ FileID getFileID(SourceLocation::UIntTy SLocOffset) const {
+ // If our one-entry cache covers this offset, just return it.
+ if (isOffsetInFileID(LastFileIDLookup, SLocOffset))
+ return LastFileIDLookup;
+
+ return getFileIDSlow(SLocOffset);
+ }
+
+ bool isLocalOffset(SourceLocation::UIntTy SLocOffset) const {
+ return SLocOffset < CurrentLoadedOffset;
+ }
+
+ bool isLoadedOffset(SourceLocation::UIntTy SLocOffset) const {
+ return SLocOffset >= CurrentLoadedOffset;
+ }
+
/// Implements the common elements of storing an expansion info struct into
/// the SLocEntry table and producing a source location that refers to it.
SourceLocation
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
