llvmorg-github-actions[bot] wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Ayokunle Amodu (ayokunle321)

<details>
<summary>Changes</summary>

This extends the source-location deduplication for module maps in #<!-- 
-->116374 to headers included textually by multiple modules.

A header included by several modules currently has its source-location entries 
serialized into each PCM. When those modules are loaded, each copy is allocated 
a separate range in Clang's source-location address space. In large `-fmodules` 
builds, this contributes to source-location space exhaustion.

This PR omits duplicate entries when writing a module. When a file is already 
provided by an imported module, the writer does not serialize another set of 
source-location entries and instead encodes those locations against the module 
that owns the existing entries.

For example:

```text
A: includes foo.h                          -o A.pcm
B: includes foo.h   -fmodule-file=A.pcm    -o B.pcm
C: includes foo.h   -fmodule-file=B.pcm    -o C.pcm
```

Previously, each PCM contained its own source-location entries for `foo.h`:

```text
A: [ ... foo.h ... ]
B: [ ... foo.h ... ]
C: [ ... foo.h ... ]
```

With this change, B and C refer to the entries from `A.pcm`:

```text
A: [ ... foo.h ... ]
B: [ ...       ... ] -&gt; A's foo.h
C: [ ...       ... ] -&gt; A's foo.h
```

Source locations are serialized as a `{module-file-index, offset}` pair, so a 
location for a redirected file can refer directly to the module that owns the 
entries.

Files are identified by path and size, taken from the `INPUT_FILE` information 
already present in the module. The lookup reads those two fields directly from 
the bitstream, lazily per module, and caches the result without materializing 
an input file or `FileEntry`.

A file whose `FileID` is serialized cannot be redirected, since a serialized 
`FileID` indexes the writing module's own file table. This includes the main 
file, files with line tables or diagnostic state, and affecting module maps 
used to unique inferred modules.

`INPUT_FILE` and `INPUT_FILE_HASH` records are still emitted for redirected 
files. Only the duplicate source-location entries are omitted.

This applies to textual includes. A module that imports another module already 
sees that module's include guards, so a later textual include of the same 
header does not create local source-location entries.

## Results

Measured on ROOT with `-Druntime_cxxmodules=On`. ROOT builds a dictionary and a 
`rootcling`-generated PCM per library, and the dictionaries repeatedly include 
shared system headers textually.

**Loaded source-location space**

75.33 MB to 61.02 MB, a reduction of 14.31 MB (19%).

The four largest contributors account for 13.35 MB of the reduction:

| header | saved |
|--------|------:|
| `c++config.h` | 4.75 MB |
| `features.h` | 2.95 MB |
| `sys/cdefs.h` | 2.72 MB |
| `assert.h` | 0.93 MB |

All four are shared non-modular system headers.

**PCM size (sum)**

377.65 MB to 362.09 MB, a reduction of 15.56 MB (4.1%).

ROOT builds with `-fmodules-embed-all-files`, so skipping an 
`SM_SLOC_FILE_ENTRY` also avoids writing its `SM_SLOC_BUFFER_BLOB`, which 
contains the file contents and accounts for nearly all of the reduction. 
Without embedded files, the PCM size reduction is around 0.4%, since the 
source-location entries themselves are small.

**RSS**

`root.exe`, which loads the modules, uses 5.4 MB less maximum RSS at startup.

`rootcling`, which writes the modules, has roughly unchanged maximum RSS. The 
import-heavy dictionaries decrease slightly, while a trivial single-import case 
increases by a few MB.

I worked on this with @<!-- -->vgvassilev.

---

Patch is 21.87 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/209795.diff


6 Files Affected:

- (modified) clang/include/clang/Serialization/ASTReader.h (+32) 
- (modified) clang/include/clang/Serialization/ASTWriter.h (+18) 
- (modified) clang/include/clang/Serialization/ModuleFile.h (+13) 
- (modified) clang/lib/Serialization/ASTReader.cpp (+87-1) 
- (modified) clang/lib/Serialization/ASTWriter.cpp (+124-38) 
- (added) clang/test/Modules/reuse-duplicate-input-file.cpp (+87) 


``````````diff
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index d800af83d350b..4e63200f09b67 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -1448,6 +1448,38 @@ class ASTReader : public ExternalPreprocessorSource,
     const StringRef &operator*() && = delete;
   };
 
+public:
+  /// Returns where a loaded module keeps the input file with path \p Path and
+  /// size \p Size, or an invalid \c FID if no loaded module has the file.
+  serialization::InputFileLoc getLoadedFileLoc(StringRef Path, off_t Size);
+
+private:
+  struct LoadedInputFile {
+    off_t Size;
+    ModuleFile *F;
+    unsigned InputID;
+  };
+
+  /// Input files of loaded modules, keyed by resolved path. Built on first 
use.
+  llvm::StringMap<SmallVector<LoadedInputFile, 1>> LoadedInputFiles;
+  bool LoadedInputFilesBuilt = false;
+
+  void buildLoadedInputFiles();
+  serialization::InputFileLoc getLoadedInputFileLoc(ModuleFile &F,
+                                                    unsigned InputID);
+
+  /// The offset of an SLoc entry and the input file it names. \c InputID is
+  /// zero for entries that are not files.
+  struct SLocEntryInfo {
+    SourceLocation::UIntTy Offset = 0;
+    unsigned InputID = 0;
+  };
+
+  /// Reads the offset and input file index from the SLoc entry at local index
+  /// \p Index in \p F.
+  llvm::Expected<SLocEntryInfo> readSLocFileEntry(ModuleFile *F,
+                                                  unsigned Index);
+
 public:
   /// Get the buffer for resolving paths.
   SmallString<0> &getPathBuf() { return PathBuf; }
diff --git a/clang/include/clang/Serialization/ASTWriter.h 
b/clang/include/clang/Serialization/ASTWriter.h
index 95ae8a6ba8c74..6bbfaad86cf51 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -542,6 +542,17 @@ class ASTWriter : public ASTDeserializationListener,
   std::vector<SourceRange> NonAffectingRanges;
   std::vector<SourceLocation::UIntTy> NonAffectingOffsetAdjustments;
 
+  /// Adjustment from a local range to the corresponding loaded range. Zero
+  /// means the range has no loaded copy.
+  ///
+  /// Unlike the adjustment vectors above, this vector is indexed by range, so
+  /// entry \c I corresponds to \c NonAffectingRanges[I].
+  std::vector<int64_t> NonAffectingRedirectAdjustments;
+
+  /// Whether the control block has been written. Import locations in the
+  /// control block must remain local.
+  bool ControlBlockWritten = false;
+
   /// A list of classes in named modules which need to emit the VTable in
   /// the corresponding object file.
   llvm::SmallVector<CXXRecordDecl *> PendingEmittingVTables;
@@ -556,6 +567,13 @@ class ASTWriter : public ASTDeserializationListener,
   SourceLocation getAffectingIncludeLoc(const SourceManager &SourceMgr,
                                         const SrcMgr::FileInfo &File);
 
+  /// Returns \p Loc in a loaded copy of its file, or an invalid location if 
the
+  /// file is kept locally.
+  SourceLocation getRedirectedLocation(SourceLocation Loc) const;
+
+  /// Returns the first non-affecting range whose end is not before \p Offset.
+  unsigned getNonAffectingRangeLowerBound(SourceLocation::UIntTy Offset) const;
+
   /// Returns an adjusted \c FileID, accounting for any non-affecting input
   /// files.
   FileID getAdjustedFileID(FileID FID) const;
diff --git a/clang/include/clang/Serialization/ModuleFile.h 
b/clang/include/clang/Serialization/ModuleFile.h
index 6c47040fde093..c05f7c8fb25e2 100644
--- a/clang/include/clang/Serialization/ModuleFile.h
+++ b/clang/include/clang/Serialization/ModuleFile.h
@@ -79,6 +79,14 @@ struct InputFileInfo {
   }
 };
 
+/// Where a module file keeps an input file. \c FID names the file and
+/// \c Offset is where its locations start. \c FID is invalid if the module
+/// file wrote no source location entries for the input file.
+struct InputFileLoc {
+  FileID FID;
+  SourceLocation::UIntTy Offset = 0;
+};
+
 /// The input file that has been loaded from this AST file, along with
 /// bools indicating whether this was an overridden buffer or if it was
 /// out-of-date or not-found.
@@ -304,6 +312,11 @@ class ModuleFile {
   /// The input file infos that have been loaded from this AST file.
   std::vector<InputFileInfo> InputFileInfosLoaded;
 
+  /// Where this module file keeps each input file. Built from source location
+  /// entries on first use.
+  std::vector<InputFileLoc> InputFileLocsLoaded;
+  bool InputFileLocsLoadedBuilt = false;
+
   // All user input files reside at the index range [0, NumUserInputFiles), and
   // system input files reside at [NumUserInputFiles, InputFilesLoaded.size()).
   unsigned NumUserInputFiles = 0;
diff --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index 3455b729be696..208a5f88f9f93 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -1817,6 +1817,14 @@ llvm::Error ASTReader::ReadSourceManagerBlock(ModuleFile 
&F) {
 
 llvm::Expected<SourceLocation::UIntTy>
 ASTReader::readSLocOffset(ModuleFile *F, unsigned Index) {
+  Expected<SLocEntryInfo> MaybeInfo = readSLocFileEntry(F, Index);
+  if (!MaybeInfo)
+    return MaybeInfo.takeError();
+  return MaybeInfo->Offset;
+}
+
+llvm::Expected<ASTReader::SLocEntryInfo>
+ASTReader::readSLocFileEntry(ModuleFile *F, unsigned Index) {
   BitstreamCursor &Cursor = F->SLocEntryCursor;
   SavedStreamPosition SavedPosition(Cursor);
   if (llvm::Error Err = Cursor.JumpToBit(F->SLocEntryOffsetsBase +
@@ -1845,10 +1853,88 @@ ASTReader::readSLocOffset(ModuleFile *F, unsigned 
Index) {
         std::errc::illegal_byte_sequence,
         "incorrectly-formatted source location entry in AST file");
   case SM_SLOC_FILE_ENTRY:
+    return SLocEntryInfo{
+        static_cast<SourceLocation::UIntTy>(F->SLocEntryBaseOffset + 
Record[0]),
+        static_cast<unsigned>(Record[4])};
   case SM_SLOC_BUFFER_ENTRY:
   case SM_SLOC_EXPANSION_ENTRY:
-    return F->SLocEntryBaseOffset + Record[0];
+    return SLocEntryInfo{
+        static_cast<SourceLocation::UIntTy>(F->SLocEntryBaseOffset + 
Record[0]),
+        0};
+  }
+}
+
+void ASTReader::buildLoadedInputFiles() {
+  LoadedInputFilesBuilt = true;
+  // ModuleManager iterates modules in index order, so the copy chosen for a
+  // file does not depend on module load order.
+  for (ModuleFile &F : ModuleMgr) {
+    for (unsigned I = 0, N = F.InputFilesLoaded.size(); I != N; ++I) {
+      InputFileInfo FI = getInputFileInfo(F, I + 1);
+      if (FI.UnresolvedImportedFilename.empty())
+        continue;
+      // An overridden input holds a buffer rather than the file named by its
+      // path, so its path and size cannot identify matching contents.
+      if (FI.Overridden)
+        continue;
+      auto Filename =
+          ResolveImportedPath(PathBuf, FI.UnresolvedImportedFilename, F);
+      // Make both paths absolute and remove dot segments before comparing 
them.
+      SmallString<128> Key(*Filename);
+      FileMgr.makeAbsolutePath(Key, /*Canonicalize=*/true);
+      LoadedInputFiles[Key].push_back({FI.StoredSize, &F, I + 1});
+    }
+  }
+}
+
+InputFileLoc ASTReader::getLoadedInputFileLoc(ModuleFile &F, unsigned InputID) 
{
+  if (!F.InputFileLocsLoadedBuilt) {
+    F.InputFileLocsLoadedBuilt = true;
+    F.InputFileLocsLoaded.resize(F.InputFilesLoaded.size());
+    for (unsigned I = 0; I != F.LocalNumSLocEntries; ++I) {
+      Expected<SLocEntryInfo> MaybeInfo = readSLocFileEntry(&F, I);
+      if (!MaybeInfo) {
+        // Failing to find an entry only prevents a redirect, so leave the file
+        // local rather than failing the write.
+        consumeError(MaybeInfo.takeError());
+        continue;
+      }
+      if (!MaybeInfo->InputID ||
+          MaybeInfo->InputID > F.InputFileLocsLoaded.size())
+        continue;
+      // A module writes its entries in order, so the first entry naming an
+      // input file is the one we want.
+      InputFileLoc &Loc = F.InputFileLocsLoaded[MaybeInfo->InputID - 1];
+      if (Loc.FID.isInvalid())
+        Loc = {FileID::get(F.SLocEntryBaseID + I), MaybeInfo->Offset};
+    }
+  }
+
+  if (InputID == 0 || InputID > F.InputFileLocsLoaded.size())
+    return InputFileLoc();
+  return F.InputFileLocsLoaded[InputID - 1];
+}
+
+InputFileLoc ASTReader::getLoadedFileLoc(StringRef Path, off_t Size) {
+  if (!LoadedInputFilesBuilt)
+    buildLoadedInputFiles();
+
+  SmallString<128> Key(Path);
+  FileMgr.makeAbsolutePath(Key, /*Canonicalize=*/true);
+  auto Known = LoadedInputFiles.find(Key);
+  if (Known == LoadedInputFiles.end())
+    return InputFileLoc();
+
+  for (const LoadedInputFile &In : Known->second) {
+    if (In.Size != Size)
+      continue;
+    // An input file may have no source location entries, leaving no copy to
+    // redirect to.
+    InputFileLoc Loc = getLoadedInputFileLoc(*In.F, In.InputID);
+    if (Loc.FID.isValid())
+      return Loc;
   }
+  return InputFileLoc();
 }
 
 int ASTReader::getSLocEntryID(SourceLocation::UIntTy SLocOffset) {
diff --git a/clang/lib/Serialization/ASTWriter.cpp 
b/clang/lib/Serialization/ASTWriter.cpp
index 9d362b1eed920..620ee0913136d 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -2919,10 +2919,12 @@ void 
ASTWriter::WritePreprocessorDetail(PreprocessingRecord &PPRec,
   if (SkippedRanges.size() > 0) {
     std::vector<PPSkippedRange> SerializedSkippedRanges;
     SerializedSkippedRanges.reserve(SkippedRanges.size());
-    for (auto const& Range : SkippedRanges)
+    for (auto const &Range : SkippedRanges) {
+      SourceRange R = getAdjustedRange(Range);
       SerializedSkippedRanges.emplace_back(
-          getRawSourceLocationEncoding(Range.getBegin()),
-          getRawSourceLocationEncoding(Range.getEnd()));
+          getRawSourceLocationEncoding(R.getBegin()),
+          getRawSourceLocationEncoding(R.getEnd()));
+    }
 
     using namespace llvm;
     auto Abbrev = std::make_shared<BitCodeAbbrev>();
@@ -5572,6 +5574,20 @@ void ASTWriter::computeNonAffectingInputFiles() {
 
   auto AffectingModuleMaps = GetAffectingModuleMaps(*PP, WritingModule);
 
+  // A FileID is serialized as an index into this module's SLoc table, so
+  // collect the local files named by records written below. The invalid FileID
+  // cannot be stored in a DenseSet, so skip it.
+  llvm::DenseSet<FileID> NamedFileIDs;
+  if (SrcMgr.getMainFileID().isValid())
+    NamedFileIDs.insert(SrcMgr.getMainFileID());
+  if (SrcMgr.hasLineTable())
+    for (const auto &L : SrcMgr.getLineTable())
+      if (L.first.ID > 0)
+        NamedFileIDs.insert(L.first);
+  for (const auto &F : PP->getDiagnostics().DiagStatesByLoc.Files)
+    if (F.first.isValid() && F.second.HasLocalTransitions)
+      NamedFileIDs.insert(F.first);
+
   unsigned FileIDAdjustment = 0;
   unsigned OffsetAdjustment = 0;
 
@@ -5581,6 +5597,33 @@ void ASTWriter::computeNonAffectingInputFiles() {
   NonAffectingFileIDAdjustments.push_back(FileIDAdjustment);
   NonAffectingOffsetAdjustments.push_back(OffsetAdjustment);
 
+  // Leaves \p FID out of this module. A nonzero \p RedirectAdjustment 
redirects
+  // its locations to a loaded copy.
+  auto MarkNonAffecting = [&](FileID FID, int64_t RedirectAdjustment) {
+    FileIDAdjustment += 1;
+    // Even empty files take up one element in the offset table.
+    OffsetAdjustment += SrcMgr.getFileIDSize(FID) + 1;
+
+    // Adjacent files with the same redirect can share a range. Files 
redirected
+    // to different copies need separate ranges.
+    if (!NonAffectingFileIDs.empty() &&
+        NonAffectingFileIDs.back().ID == FID.ID - 1 &&
+        NonAffectingRedirectAdjustments.back() == RedirectAdjustment) {
+      NonAffectingFileIDs.back() = FID;
+      NonAffectingRanges.back().setEnd(SrcMgr.getLocForEndOfFile(FID));
+      NonAffectingFileIDAdjustments.back() = FileIDAdjustment;
+      NonAffectingOffsetAdjustments.back() = OffsetAdjustment;
+      return;
+    }
+
+    NonAffectingFileIDs.push_back(FID);
+    NonAffectingRanges.emplace_back(SrcMgr.getLocForStartOfFile(FID),
+                                    SrcMgr.getLocForEndOfFile(FID));
+    NonAffectingFileIDAdjustments.push_back(FileIDAdjustment);
+    NonAffectingOffsetAdjustments.push_back(OffsetAdjustment);
+    NonAffectingRedirectAdjustments.push_back(RedirectAdjustment);
+  };
+
   for (unsigned I = 1; I != N; ++I) {
     const SrcMgr::SLocEntry *SLoc = &SrcMgr.getLocalSLocEntry(I);
     FileID FID = FileID::get(I);
@@ -5593,44 +5636,45 @@ void ASTWriter::computeNonAffectingInputFiles() {
     if (!Cache->OrigEntry)
       continue;
 
-    // Don't prune anything other than module maps.
-    if (!isModuleMap(File.getFileCharacteristic()))
-      continue;
+    if (isModuleMap(File.getFileCharacteristic())) {
+      // Don't prune module maps if all are guaranteed to be affecting.
+      if (!AffectingModuleMaps)
+        continue;
 
-    // Don't prune module maps if all are guaranteed to be affecting.
-    if (!AffectingModuleMaps)
-      continue;
+      // Affecting module maps may be named by FileID in the submodule block, 
so
+      // they cannot be redirected.
+      if (AffectingModuleMaps->DefinitionFileIDs.contains(FID))
+        continue;
 
-    // Don't prune module maps that are affecting.
-    if (AffectingModuleMaps->DefinitionFileIDs.contains(FID))
+      // A module map with no affecting locations can be left out.
+      IsSLocAffecting[I] = false;
+      IsSLocFileEntryAffecting[I] =
+          AffectingModuleMaps->DefinitionFiles.contains(*Cache->OrigEntry);
+      MarkNonAffecting(FID, 0);
       continue;
+    }
 
-    IsSLocAffecting[I] = false;
-    IsSLocFileEntryAffecting[I] =
-        AffectingModuleMaps->DefinitionFiles.contains(*Cache->OrigEntry);
-
-    FileIDAdjustment += 1;
-    // Even empty files take up one element in the offset table.
-    OffsetAdjustment += SrcMgr.getFileIDSize(FID) + 1;
+    if (NamedFileIDs.contains(FID))
+      continue;
 
-    // If the previous file was non-affecting as well, just extend its entry
-    // with our information.
-    if (!NonAffectingFileIDs.empty() &&
-        NonAffectingFileIDs.back().ID == FID.ID - 1) {
-      NonAffectingFileIDs.back() = FID;
-      NonAffectingRanges.back().setEnd(SrcMgr.getLocForEndOfFile(FID));
-      NonAffectingFileIDAdjustments.back() = FileIDAdjustment;
-      NonAffectingOffsetAdjustments.back() = OffsetAdjustment;
+    // Reuse the source location entries of a loaded module that already has
+    // this input file.
+    if (!hasChain())
+      continue;
+    serialization::InputFileLoc Loaded = getChain()->getLoadedFileLoc(
+        Cache->OrigEntry->getName(), Cache->OrigEntry->getSize());
+    if (Loaded.FID.isInvalid())
       continue;
-    }
 
-    NonAffectingFileIDs.push_back(FID);
-    NonAffectingRanges.emplace_back(SrcMgr.getLocForStartOfFile(FID),
-                                    SrcMgr.getLocForEndOfFile(FID));
-    NonAffectingFileIDAdjustments.push_back(FileIDAdjustment);
-    NonAffectingOffsetAdjustments.push_back(OffsetAdjustment);
+    IsSLocAffecting[I] = false;
+    IsSLocFileEntryAffecting[I] = true;
+    MarkNonAffecting(FID, static_cast<int64_t>(SLoc->getOffset()) -
+                              static_cast<int64_t>(Loaded.Offset));
   }
 
+  assert(NonAffectingRedirectAdjustments.size() == NonAffectingRanges.size() &&
+         "Every non-affecting range needs a redirect adjustment");
+
   if (!PP->getHeaderSearchInfo().getHeaderSearchOpts().ModulesIncludeVFSUsage)
     return;
 
@@ -6152,6 +6196,9 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema *SemaPtr, 
StringRef isysroot,
 
   // Write the control block
   WriteControlBlock(*PP, isysroot);
+  // Import locations in the control block must remain local, so start 
rewriting
+  // only after it has been written.
+  ControlBlockWritten = true;
 
   // Write the remaining AST contents.
   Stream.FlushToWord();
@@ -6770,6 +6817,9 @@ FileID ASTWriter::getAdjustedFileID(FileID FID) const {
   if (FID.isInvalid() || PP->getSourceManager().isLoadedFileID(FID) ||
       NonAffectingFileIDs.empty())
     return FID;
+  
assert(getRedirectedLocation(PP->getSourceManager().getLocForStartOfFile(FID))
+             .isInvalid() &&
+         "Cannot name a redirected file by FileID");
   auto It = llvm::lower_bound(NonAffectingFileIDs, FID);
   unsigned Idx = std::distance(NonAffectingFileIDs.begin(), It);
   unsigned Offset = NonAffectingFileIDAdjustments[Idx];
@@ -6789,9 +6839,39 @@ unsigned ASTWriter::getAdjustedNumCreatedFIDs(FileID 
FID) const {
   return AdjustedNumCreatedFIDs;
 }
 
+SourceLocation ASTWriter::getRedirectedLocation(SourceLocation Loc) const {
+  if (NonAffectingRedirectAdjustments.empty())
+    return SourceLocation();
+
+  SourceLocation::UIntTy Offset = Loc.getOffset();
+  if (PP->getSourceManager().isLoadedOffset(Offset))
+    return SourceLocation();
+
+  unsigned Idx = getNonAffectingRangeLowerBound(Offset);
+  if (Idx == NonAffectingRanges.size())
+    return SourceLocation();
+
+  // The search only rules out ranges ending before the offset, so check that
+  // the offset really is inside the one we landed on.
+  if (Offset < NonAffectingRanges[Idx].getBegin().getOffset())
+    return SourceLocation();
+
+  int64_t Adjustment = NonAffectingRedirectAdjustments[Idx];
+  if (!Adjustment)
+    return SourceLocation();
+  return SourceLocation::getFileLoc(static_cast<SourceLocation::UIntTy>(
+      static_cast<int64_t>(Offset) - Adjustment));
+}
+
 SourceLocation ASTWriter::getAdjustedLocation(SourceLocation Loc) const {
   if (Loc.isInvalid())
     return Loc;
+  // Redirect locations in omitted files before adjusting local offsets.
+  // getAdjustment() is also used for values that are not source locations.
+  if (ControlBlockWritten && !Loc.isMacroID())
+    if (SourceLocation Redirected = getRedirectedLocation(Loc);
+        Redirected.isValid())
+      return Redirected;
   return Loc.getLocWithOffset(-getAdjustment(Loc.getOffset()));
 }
 
@@ -6819,13 +6899,17 @@ ASTWriter::getAdjustment(SourceLocation::UIntTy Offset) 
const {
   if (Offset < NonAffectingRanges.front().getBegin().getOffset())
     return 0;
 
-  auto Contains = [](const SourceRange &Range, SourceLocation::UIntTy Offset) {
+  return NonAffectingOffsetAdjustments[getNonAffectingRangeLowerBound(Offset)];
+}
+
+unsigned
+ASTWriter::getNonAffectingRangeLowerBound(SourceLocation::UIntTy Offset) const 
{
+  auto EndsBefore = [](const SourceRange &Range,
+                       SourceLocation::UIntTy Offset) {
     return Range.getEnd().getOffset() < Offset;
   };
-
-  auto It = llvm::lower_bound(NonAffectingRanges, Offset, Contains);
-  unsigned Idx = std::distance(NonAffectingRanges.begin(), It);
-  return NonAffectingOffsetAdjustments[Idx];
+  auto It = llvm::lower_bound(NonAffectingRanges, Offset, EndsBefore);
+  return std::distance(NonAffectingRanges.begin(), It);
 }
 
 void ASTWriter::AddFileID(FileID FID, RecordDataImpl &Record) {
@@ -7186,7 +7270,9 @@ void ASTWriter::associateDeclWithFile(const Decl *D, 
LocalDeclID ID) {
   if (FID.isInvalid())
     return;
   assert(SM.getSLocEntry(FID).isFile());
-  assert(IsSLocAffecting[FID.ID]);
+  // A redirected file already has its declaration table in the loaded module.
+  if (!IsSLocAffecting[FID.ID])
+    return;
 
   std::unique_ptr<DeclIDInFileInfo> &Info = FileDeclIDs[FID];
   if (!Info)
diff --git a/clang/test/Modules/reuse-duplicate-input-file.cpp 
b/clang/test/Modules/reuse-duplicate-input-file.cpp
new file mode 100644
index 0000000000000..bacb5077f00d8
--- /dev/null
+++ b/clang/test/Modules/reuse-duplicate-input-file.cpp
@@ -0,0 +1,87 @@
+// Check that a header included textually by several modules reuses source
+// location entries, while headers named by FileID do not.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -xc++ -fmodules -fno-implicit-modules \
+// RUN:   -fmodule-map-file=%t/mods.map \
+// RUN:   -fmodule-name=mod1 -emit-module %t/mods.map -o %t/mod1.pcm
+// RUN: %clang_cc1 -xc++ -fmodules -fno-implicit-modules \
+// RUN:   -fmodule-map-file=%t/mods.map -fmodule-file=%t/mod1.pcm \
+// RUN:   -fmodule-name=mod2 -emit-module %t/mods.map -o %t/mod2.pcm
+// RUN: %clang_cc1 -xc++ -fmodules -fno-implicit-modules \
+// RUN:   -fmodule-map-file=%t/mods.map -f...
[truncated]

``````````

</details>


https://github.com/llvm/llvm-project/pull/209795
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to