jansvoboda11 updated this revision to Diff 471641.
jansvoboda11 added a comment.
Avoid looking up `FileID` for an offset.
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,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,9 @@
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.
+ // Do not emit inputs that do not affect current module.
+ if (!IsSLocAffecting[I])
continue;
- }
InputFileEntry Entry;
Entry.File = Cache->OrigEntry;
@@ -2046,7 +2032,6 @@
// Record the offset of this source-location entry.
uint64_t Offset = Stream.GetCurrentBitNo() - SLocEntryOffsetsBase;
assert((Offset >> 32) == 0 && "SLocEntry offset too large");
- SLocEntryOffsets.push_back(Offset);
// Figure out which record code to use.
unsigned Code;
@@ -2061,17 +2046,15 @@
Record.clear();
Record.push_back(Code);
- // Starting offset of this entry within this module, so skip the dummy.
- Record.push_back(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.
+ // Do not emit files that were not listed as inputs.
+ if (!IsSLocAffecting[I])
continue;
- }
+ SLocEntryOffsets.push_back(Offset);
+ // Starting offset of this entry within this module, so skip the dummy.
+ Record.push_back(getAdjustedOffset(SLoc->getOffset()) - 2);
AddSourceLocation(File.getIncludeLoc(), Record);
Record.push_back(File.getFileCharacteristic()); // FIXME: stable encoding
Record.push_back(File.hasLineDirectives());
@@ -2133,6 +2116,9 @@
// The source location entry is a macro expansion.
const SrcMgr::ExpansionInfo &Expansion = SLoc->getExpansion();
LocSeq::State Seq;
+ SLocEntryOffsets.push_back(Offset);
+ // Starting offset of this entry within this module, so skip the dummy.
+ Record.push_back(getAdjustedOffset(SLoc->getOffset()) - 2);
AddSourceLocation(Expansion.getSpellingLoc(), Record, Seq);
AddSourceLocation(Expansion.getExpansionLocStart(), Record, Seq);
AddSourceLocation(Expansion.isMacroArgExpansion()
@@ -2145,7 +2131,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 +2155,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));
@@ -2206,7 +2192,7 @@
continue;
// Emit the file ID
- Record.push_back(L.first.ID);
+ AddFileID(L.first, Record);
// Emit the line entries
Record.push_back(L.second.size());
@@ -2569,7 +2555,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 +2996,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 +4513,69 @@
}
}
+void ASTWriter::collectNonAffectingInputFiles() {
+ SourceManager &SrcMgr = PP->getSourceManager();
+ unsigned N = SrcMgr.local_sloc_entry_size();
+
+ IsSLocAffecting.resize(N, true);
+
+ if (!WritingModule)
+ return;
+
+ auto AffectingModuleMaps =
+ GetAllModuleMaps(PP->getHeaderSearchInfo(), WritingModule);
+
+ unsigned Size = 0;
+ unsigned Count = 0;
+
+ NonAffectingInputOffsetAdjustments.reserve(N);
+ NonAffectingFileIDsAdjustments.reserve(N);
+
+ NonAffectingInputOffsetAdjustments.push_back(Size);
+ NonAffectingFileIDsAdjustments.push_back(Count);
+
+ 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);
+
+ 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()) {
+ IsSLocAffecting[I] = false;
+
+ Size += SrcMgr.getFileIDSize(FID);
+ Count += 1;
+
+ if (!NonAffectingFileIDs.empty() &&
+ NonAffectingFileIDs.back().ID == FID.ID - 1) {
+ // If the previous file was non-affecting as well, just extend its entry
+ // with our information.
+ NonAffectingFileIDs.back() = FID;
+ NonAffectingRanges.back().setEnd(SrcMgr.getLocForEndOfFile(FID));
+ NonAffectingInputOffsetAdjustments.back() = Size;
+ NonAffectingFileIDsAdjustments.back() = Count;
+ } else {
+ NonAffectingFileIDs.push_back(FID);
+ NonAffectingRanges.emplace_back(SrcMgr.getLocForStartOfFile(FID),
+ SrcMgr.getLocForEndOfFile(FID));
+ NonAffectingInputOffsetAdjustments.push_back(Size);
+ NonAffectingFileIDsAdjustments.push_back(Count);
+ }
+ }
+ }
+}
+
ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot,
Module *WritingModule) {
using namespace llvm;
@@ -4539,6 +4589,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 +5279,51 @@
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) ||
+ NonAffectingFileIDs.empty())
+ return FID;
+ auto It = llvm::lower_bound(NonAffectingFileIDs, FID);
+ unsigned Idx = std::distance(NonAffectingFileIDs.begin(), It);
+ unsigned Offset = NonAffectingFileIDsAdjustments[Idx];
+ return FileID::get(FID.getOpaqueValue() - Offset);
+}
+
+SourceLocation::UIntTy
+ASTWriter::getAdjustmentSlow(SourceLocation::UIntTy Offset) const {
+ auto RangeContainsOffset = [](const SourceRange &Range,
+ SourceLocation::UIntTy Offset) {
+ return Range.getEnd().getOffset() < Offset;
+ };
+
+ auto It = llvm::lower_bound(NonAffectingRanges, Offset, RangeContainsOffset);
+ unsigned Idx = std::distance(NonAffectingRanges.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
@@ -20,6 +20,8 @@
#include "clang/Basic/LLVM.h"
#include "clang/Basic/Module.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Lex/Preprocessor.h"
#include "clang/Sema/Sema.h"
#include "clang/Sema/SemaConsumer.h"
#include "clang/Serialization/ASTBitCodes.h"
@@ -444,8 +446,48 @@
std::vector<std::unique_ptr<ModuleFileExtensionWriter>>
ModuleFileExtensionWriters;
- /// User ModuleMaps skipped when writing control block.
- std::set<const FileEntry *> SkippedModuleMaps;
+ /// Mapping from a source location entry to whether it is affecting or not.
+ llvm::BitVector IsSLocAffecting;
+
+ /// Mapping from \c FileID to an index into the FileID adjustment table.
+ std::vector<FileID> NonAffectingFileIDs;
+ std::vector<unsigned> NonAffectingFileIDsAdjustments;
+
+ /// Mapping from an offset to an index into the offset adjustment table.
+ std::vector<SourceRange> NonAffectingRanges;
+ std::vector<SourceLocation::UIntTy> NonAffectingInputOffsetAdjustments;
+
+ /// 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 {
+ if (NonAffectingRanges.empty())
+ return 0;
+
+ if (PP->getSourceManager().isLoadedOffset(Offset))
+ return 0;
+
+ if (Offset < NonAffectingRanges.front().getBegin().getOffset())
+ return 0;
+
+ if (NonAffectingRanges.back().getEnd().getOffset() < Offset)
+ return NonAffectingInputOffsetAdjustments.back();
+
+ return getAdjustmentSlow(Offset);
+ }
+ SourceLocation::UIntTy getAdjustmentSlow(SourceLocation::UIntTy Offset) const;
/// Retrieve or create a submodule ID for this module.
unsigned getSubmoduleID(Module *Mod);
@@ -465,8 +507,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 +623,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
