jansvoboda11 updated this revision to Diff 471152.
jansvoboda11 added a comment.
New version with flat vector + `FileID` indices; replacing the previous compact
representation & binary search
approach
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136624/new/
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
@@ -1468,12 +1468,12 @@
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;
@@ -1483,8 +1483,7 @@
}
WriteInputFiles(Context.SourceMgr,
- PP.getHeaderSearchInfo().getHeaderSearchOpts(),
- AffectingModuleMaps);
+ PP.getHeaderSearchInfo().getHeaderSearchOpts());
Stream.ExitBlock();
}
@@ -1502,9 +1501,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);
@@ -1536,6 +1534,10 @@
const SrcMgr::SLocEntry *SLoc = &SourceMgr.getLocalSLocEntry(I);
assert(&SourceMgr.getSLocEntry(FileID::get(I)) == SLoc);
+ // Do not emit inputs that do not affect current module.
+ if (!SLocEntryInfos[I].Affecting)
+ continue;
+
// We only care about file entries that were not overridden.
if (!SLoc->isFile())
continue;
@@ -1544,16 +1546,6 @@
if (!Cache->OrigEntry)
continue;
- 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.
- continue;
- }
-
InputFileEntry Entry;
Entry.File = Cache->OrigEntry;
Entry.IsSystemFile = isSystem(File.getFileCharacteristic());
@@ -2043,6 +2035,10 @@
FileID FID = FileID::get(I);
assert(&SourceMgr.getSLocEntry(FID) == SLoc);
+ // Do not emit files that were not listed as inputs.
+ if (!SLocEntryInfos[I].Affecting)
+ continue;
+
// Record the offset of this source-location entry.
uint64_t Offset = Stream.GetCurrentBitNo() - SLocEntryOffsetsBase;
assert((Offset >> 32) == 0 && "SLocEntry offset too large");
@@ -2062,16 +2058,11 @@
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()) {
- // Do not emit files that were not listed as inputs.
- continue;
- }
+
AddSourceLocation(File.getIncludeLoc(), Record);
Record.push_back(File.getFileCharacteristic()); // FIXME: stable encoding
Record.push_back(File.hasLineDirectives());
@@ -2145,7 +2136,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 +2160,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 +2560,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 +3001,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 +4518,52 @@
}
}
+void ASTWriter::collectNonAffectingInputFiles() {
+ SourceManager &SrcMgr = PP->getSourceManager();
+ unsigned N = SrcMgr.local_sloc_entry_size();
+
+ if (!WritingModule) {
+ SLocEntryInfos.resize(N); // all entries affecting
+ return;
+ }
+
+ SLocEntryInfos.reserve(N);
+ SLocEntryInfos.emplace_back(); // dummy
+
+ std::set<const FileEntry *> AffectingModuleMaps = GetAffectingModuleMaps(
+ PP->getHeaderSearchInfo(), PP->getSourceManager(), WritingModule);
+
+ unsigned NonAffectingCount = 0;
+ unsigned NonAffectingSize = 0;
+
+ for (unsigned I = 1; I != N; ++I) {
+ // Get this source location entry.
+ const SrcMgr::SLocEntry *SLoc = &SrcMgr.getLocalSLocEntry(I);
+
+ FileID FID = FileID::get(I);
+ assert(&SrcMgr.getSLocEntry(FID) == SLoc);
+
+ SLocEntryInfos.emplace_back(NonAffectingCount, NonAffectingSize);
+
+ 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()) {
+ NonAffectingCount += 1;
+ NonAffectingSize += SrcMgr.getFileIDSize(FID);
+ SLocEntryInfos.back().markNonAffecting();
+ }
+ }
+}
+
ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot,
Module *WritingModule) {
using namespace llvm;
@@ -4539,6 +4577,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 +5267,45 @@
Record.push_back(Raw);
}
+void ASTWriter::AddFileID(FileID FID, RecordDataImpl &Record) {
+ Record.push_back(getAdjustedFileID(FID).getOpaqueValue());
+}
+
+FileID ASTWriter::getAdjustedFileID(FileID FID) const {
+ if (PP->getSourceManager().isLoadedFileID(FID))
+ return FID;
+ return FileID::get(FID.ID - SLocEntryInfos[FID.ID].FileIDAdjustment);
+}
+
+SourceLocation::UIntTy
+ASTWriter::getAdjustment(SourceLocation::UIntTy Offset) const {
+ if (PP->getSourceManager().isLoadedOffset(Offset))
+ return 0;
+ if (Offset == PP->getSourceManager().getNextLocalOffset())
+ return SLocEntryInfos.back().OffsetAdjustment;
+ FileID FID = PP->getSourceManager().getFileID(Offset);
+ return SLocEntryInfos[FID.ID].OffsetAdjustment;
+}
+
+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,38 @@
std::vector<std::unique_ptr<ModuleFileExtensionWriter>>
ModuleFileExtensionWriters;
- /// User ModuleMaps skipped when writing control block.
- std::set<const FileEntry *> SkippedModuleMaps;
+ struct SLocEntryInfo {
+ unsigned Affecting : 1;
+ unsigned FileIDAdjustment : 31;
+ unsigned OffsetAdjustment : 32;
+
+ SLocEntryInfo() : Affecting(1), FileIDAdjustment(0), OffsetAdjustment(0) {}
+ SLocEntryInfo(unsigned FileIDAdjustment, unsigned OffsetAdjustment)
+ : Affecting(1), FileIDAdjustment(FileIDAdjustment),
+ OffsetAdjustment(OffsetAdjustment) {}
+
+ void markNonAffecting() { Affecting = false; }
+ };
+
+ /// Sorted local source location entry information.
+ std::vector<SLocEntryInfo> SLocEntryInfos;
+
+ /// 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 +495,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 +611,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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
