[clang] Update code that assumes FileEntry::getName is absolute NFC

[benlangmuir]

Summary

It's an accident that we started return asbolute paths from FileEntry::getName for all relative paths. Prepare for getName to get (closer to) return the requested path. Note: conceptually it might make sense for the dependency scanner to allow relative paths and have the DependencyConsumer decide if it wants to make them absolute, but we currently document that it's absolute and I didn't want to change behaviour here.

[bnbarham]

LGTM, thanks for doing this 🙇

[fmayer]

This CL seems to have an MSAN violation: https://lab.llvm.org/buildbot/#/builders/74/builds/12529/steps/11/logs/stdio

[benlangmuir]

@fmayer thanks, fix is: https://reviews.llvm.org/D131017

clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp

llvm::DenseMap<const FileEntry *,
                  std::map<tooling::Replacement,
                           const tooling::TranslationUnitDiagnostics *>>
       DiagReplacements;
 
   auto AddToGroup = [&](const tooling::Replacement &R,
                         const tooling::TranslationUnitDiagnostics *SourceTU,
                         const llvm::Optional<std::string> BuildDir) {
     // Use the file manager to deduplicate paths. FileEntries are
-    // automatically canonicalized.
-    auto PrevWorkingDir = SM.getFileManager().getFileSystemOpts().WorkingDir;
+    // automatically canonicalized. Since relative paths can come from different
+    // build directories, make them absolute immediately.
+    SmallString<128> Path = R.getFilePath();
     if (BuildDir)
-      SM.getFileManager().getFileSystemOpts().WorkingDir = std::move(*BuildDir);
-    if (auto Entry = SM.getFileManager().getFile(R.getFilePath())) {
+      llvm::sys::fs::make_absolute(*BuildDir, Path);
+    else
+      SM.getFileManager().makeAbsolutePath(Path);
+
+    if (auto Entry = SM.getFileManager().getFile(Path)) {
       if (SourceTU) {
         auto &Replaces = DiagReplacements[*Entry];
         auto It = Replaces.find(R);
         if (It == Replaces.end())
           Replaces.emplace(R, SourceTU);
         else if (It->second != SourceTU)
           // This replacement is a duplicate of one suggested by another TU.
           return;
       }
       GroupedReplacements[*Entry].push_back(R);
-    } else if (Warned.insert(R.getFilePath()).second) {
+    } else if (Warned.insert(Path).second) {
       errs() << "Described file '" << R.getFilePath()
              << "' doesn't exist. Ignoring...\n";
     }
-    SM.getFileManager().getFileSystemOpts().WorkingDir = PrevWorkingDir;
   };
 
   for (const auto &TU : TUs)
     for (const tooling::Replacement &R : TU.Replacements)
       AddToGroup(R, nullptr, {});
 
   for (const auto &TU : TUDs)
     for (const auto &D : TU.Diagnostics)

clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h

private:
   /// The original Clang invocation passed to dependency scanner.
   CompilerInvocation OriginalInvocation;
   /// Whether to optimize the modules' command-line arguments.
   bool OptimizeArgs;
 
   /// Checks whether the module is known as being prebuilt.
   bool isPrebuiltModule(const Module *M);
 
+  /// Adds \p Path to \c FileDeps, making it absolute if necessary.
+  void addFileDep(StringRef Path);
+  /// Adds \p Path to \c MD.FileDeps, making it absolute if necessary.
+  void addFileDep(ModuleDeps &MD, StringRef Path);
+
   /// Constructs a CompilerInvocation that can be used to build the given
   /// module, excluding paths to discovered modular dependencies that are yet to
   /// be built.
   CompilerInvocation makeInvocationForModuleBuildWithoutPaths(
       const ModuleDeps &Deps,
       llvm::function_ref<void(CompilerInvocation &)> Optimize) const;
 };
 
 } // end namespace dependencies
 } // end namespace tooling
 } // end namespace clang
 
 #endif // LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_MODULEDEPCOLLECTOR_H

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

 using namespace dependencies;
 
 namespace {
 
 /// Forwards the gatherered dependencies to the consumer.
 class DependencyConsumerForwarder : public DependencyFileGenerator {
 public:
   DependencyConsumerForwarder(std::unique_ptr<DependencyOutputOptions> Opts,
-                              DependencyConsumer &C)
-      : DependencyFileGenerator(*Opts), Opts(std::move(Opts)), C(C) {}
+                              StringRef WorkingDirectory, DependencyConsumer &C)
+      : DependencyFileGenerator(*Opts), WorkingDirectory(WorkingDirectory),
+        Opts(std::move(Opts)), C(C) {}
 
   void finishedMainFile(DiagnosticsEngine &Diags) override {
     C.handleDependencyOutputOpts(*Opts);
     llvm::SmallString<256> CanonPath;
     for (const auto &File : getDependencies()) {
       CanonPath = File;
       llvm::sys::path::remove_dots(CanonPath, /*remove_dot_dot=*/true);
+      llvm::sys::fs::make_absolute(WorkingDirectory, CanonPath);
       C.handleFileDependency(CanonPath);
     }
   }
 
 private:
+  StringRef WorkingDirectory;
   std::unique_ptr<DependencyOutputOptions> Opts;
   DependencyConsumer &C;
 };
 
 using PrebuiltModuleFilesT = decltype(HeaderSearchOptions::PrebuiltModuleFiles);
 
 /// A listener that collects the imported modules and optionally the input
 /// files.
if (Opts->Targets.empty())
       Opts->Targets = {
           deduceDepTarget(ScanInstance.getFrontendOpts().OutputFile,
                           ScanInstance.getFrontendOpts().Inputs)};
     Opts->IncludeSystemHeaders = true;
 
     switch (Format) {
     case ScanningOutputFormat::Make:
       ScanInstance.addDependencyCollector(
-          std::make_shared<DependencyConsumerForwarder>(std::move(Opts),
-                                                        Consumer));
+          std::make_shared<DependencyConsumerForwarder>(
+              std::move(Opts), WorkingDirectory, Consumer));
       break;
     case ScanningOutputFormat::Full:
       ScanInstance.addDependencyCollector(std::make_shared<ModuleDepCollector>(
           std::move(Opts), ScanInstance, Consumer,
           std::move(OriginalInvocation), OptimizeArgs));
       break;
     }
 

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

void ModuleDepCollectorPP::FileChanged(SourceLocation Loc,
 
   SourceManager &SM = MDC.ScanInstance.getSourceManager();
 
   // Dependency generation really does want to go all the way to the
   // file entry for a source location to find out what is depended on.
   // We do not want #line markers to affect dependency generation!
   if (Optional<StringRef> Filename =
           SM.getNonBuiltinFilenameForID(SM.getFileID(SM.getExpansionLoc(Loc))))
-    MDC.FileDeps.push_back(
-        std::string(llvm::sys::path::remove_leading_dotslash(*Filename)));
+    MDC.addFileDep(llvm::sys::path::remove_leading_dotslash(*Filename));
 }
 
 void ModuleDepCollectorPP::InclusionDirective(
     SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
     bool IsAngled, CharSourceRange FilenameRange, Optional<FileEntryRef> File,
     StringRef SearchPath, StringRef RelativePath, const Module *Imported,
     SrcMgr::CharacteristicKind FileType) {
   if (!File && !Imported) {
     // This is a non-modular include that HeaderSearch failed to find. Add it
     // here as `FileChanged` will never see it.
-    MDC.FileDeps.push_back(std::string(FileName));
+    MDC.addFileDep(FileName);
   }
   handleImport(Imported);
 }
 
 void ModuleDepCollectorPP::moduleImport(SourceLocation ImportLoc,
                                         ModuleIdPath Path,
                                         const Module *Imported) {
   handleImport(Imported);

 
 void ModuleDepCollectorPP::EndOfMainFile() {
   FileID MainFileID = MDC.ScanInstance.getSourceManager().getMainFileID();
   MDC.MainFile = std::string(MDC.ScanInstance.getSourceManager()
                                  .getFileEntryForID(MainFileID)
                                  ->getName());
 
   if (!MDC.ScanInstance.getPreprocessorOpts().ImplicitPCHInclude.empty())
-    MDC.FileDeps.push_back(
-        MDC.ScanInstance.getPreprocessorOpts().ImplicitPCHInclude);
+    MDC.addFileDep(MDC.ScanInstance.getPreprocessorOpts().ImplicitPCHInclude);
 
   for (const Module *M : DirectModularDeps) {
     // A top-level module might not be actually imported as a module when
     // -fmodule-name is used to compile a translation unit that imports this
     // module. In that case it can be skipped. The appropriate header
     // dependencies will still be reported as expected.
     if (!M->getASTFile())
       continue;
ModuleID ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
   MDC.ScanInstance.getASTReader()->visitInputFiles(
       *MF, true, true, [&](const serialization::InputFile &IF, bool isSystem) {
         // __inferred_module.map is the result of the way in which an implicit
         // module build handles inferred modules. It adds an overlay VFS with
         // this file in the proper directory and relies on the rest of Clang to
         // handle it like normal. With explicitly built modules we don't need
         // to play VFS tricks, so replace it with the correct module map.
         if (IF.getFile()->getName().endswith("__inferred_module.map")) {
-          MD.FileDeps.insert(ModuleMap->getName());
+          MDC.addFileDep(MD, ModuleMap->getName());
           return;
         }
-        MD.FileDeps.insert(IF.getFile()->getName());
+        MDC.addFileDep(MD, IF.getFile()->getName());
       });
 
   // We usually don't need to list the module map files of our dependencies when
   // building a module explicitly: their semantics will be deserialized from PCM
   // files.
   //
   // However, some module maps loaded implicitly during the dependency scan can
   // describe anti-dependencies. That happens when this module, let's call it
const auto &PrebuiltModuleFiles =
       ScanInstance.getHeaderSearchOpts().PrebuiltModuleFiles;
   auto PrebuiltModuleFileIt = PrebuiltModuleFiles.find(Name);
   if (PrebuiltModuleFileIt == PrebuiltModuleFiles.end())
     return false;
   assert("Prebuilt module came from the expected AST file" &&
          PrebuiltModuleFileIt->second == M->getASTFile()->getName());
   return true;
 }
+
+static StringRef makeAbsolute(CompilerInstance &CI, StringRef Path,
+                              SmallVectorImpl<char> &Storage) {
+  if (llvm::sys::path::is_absolute(Path))
+    return Path;
+  Storage.assign(Path.begin(), Path.end());
+  CI.getFileManager().makeAbsolutePath(Storage);
+  return StringRef(Storage.data(), Storage.size());
+}
+
+void ModuleDepCollector::addFileDep(StringRef Path) {
+  llvm::SmallString<256> Storage;
+  Path = makeAbsolute(ScanInstance, Path, Storage);
+  FileDeps.push_back(std::string(Path));
+}
+
+void ModuleDepCollector::addFileDep(ModuleDeps &MD, StringRef Path) {
+  llvm::SmallString<256> Storage;
+  Path = makeAbsolute(ScanInstance, Path, Storage);
+  MD.FileDeps.insert(Path);
+}