Keep configuration file search directories in ExpansionContext. NFC

[sepavloff]

Summary

Class ExpansionContext encapsulates options for search and expansion of
response files, including configuration files. With this change the
directories which are searched for configuration files are also stored
in ExpansionContext.

[mgorny]

clang/lib/Driver/Driver.cpp

1020

I don't really understand the purpose of this change, or how it's related to moving search dirs.

[mgorny]

clang/lib/Driver/Driver.cpp

1020

Oh, it's because of ExpCtx.findConfigFile() API.

[mgorny]

llvm/include/llvm/Support/CommandLine.h

2197

I'm sorry if I'm missing something obvious but is there a specific reason you're using std::string here when pretty much all prior call sites were using SmallString?

[sepavloff]

Change parameter type of findConfigFile, rebased

[sepavloff]

llvm/include/llvm/Support/CommandLine.h

2197

Changed type of this parameter to base of SmallString.

[mgorny]

I haven't tested it but eyeball-review, it looks neat. Thanks!

[sepavloff]

Thanks!

[MaskRay]

I am not familiar with ExpansionContext. Is there any behavior difference which can be demonstrated by a test?

[sepavloff]

In D135439#3864165, @MaskRay wrote:

I am not familiar with ExpansionContext. Is there any behavior difference which can be demonstrated by a test?

This is an NFC test, it does not change behavior. However moving these properties into ExpansionContext provides access to the search directories in the functions expandResponseFile(s) and allows more natural implementation of the functionality of D133325.

clang/include/clang/Driver/Driver.h

 #include <string>
 #include <vector>
 
 namespace llvm {
 class Triple;
 namespace vfs {
 class FileSystem;
 }
+namespace cl {
+class ExpansionContext;
+}
 } // namespace llvm
 
 namespace clang {
 
 namespace driver {
 
 typedef SmallVector<InputInfo, 4> InputInfoList;
 
private:
   ///
   /// \returns true if error occurred.
   bool loadConfigFiles();
 
   /// Tries to load options from default configuration files (deduced from
   /// executable filename).
   ///
   /// \returns true if error occurred.
-  bool loadDefaultConfigFiles(ArrayRef<StringRef> CfgFileSearchDirs);
+  bool loadDefaultConfigFiles(llvm::cl::ExpansionContext &ExpCtx);
 
   /// Read options from the specified file.
   ///
   /// \param [in] FileName File to read.
+  /// \param [in] Search and expansion options.
   /// \returns true, if error occurred while reading.
-  bool readConfigFile(StringRef FileName);
+  bool readConfigFile(StringRef FileName, llvm::cl::ExpansionContext &ExpCtx);
 
   /// Set the driver mode (cl, gcc, etc) from the value of the `--driver-mode`
   /// option.
   void setDriverMode(StringRef DriverModeValue);
 
   /// Parse the \p Args list for LTO options and record the type of LTO
   /// compilation based on which -f(no-)?lto(=.*)? option occurs last.
   void setLTOMode(const llvm::opt::ArgList &Args);

clang/lib/Driver/Driver.cpp

if (IsOpenMPOffloading) {
     return;
   }
 
   //
   // TODO: Add support for other offloading programming models here.
   //
 }
 
-/// Looks the given directories for the specified file.
-///
-/// \param[out] FilePath File path, if the file was found.
-/// \param[in]  Dirs Directories used for the search.
-/// \param[in]  FileName Name of the file to search for.
-/// \return True if file was found.
-///
-/// Looks for file specified by FileName sequentially in directories specified
-/// by Dirs.
-///
-static bool searchForFile(SmallVectorImpl<char> &FilePath,
-                          ArrayRef<StringRef> Dirs, StringRef FileName,
-                          llvm::vfs::FileSystem &FS) {
-  SmallString<128> WPath;
-  for (const StringRef &Dir : Dirs) {
-    if (Dir.empty())
-      continue;
-    WPath.clear();
-    llvm::sys::path::append(WPath, Dir, FileName);
-    llvm::sys::path::native(WPath);
-    auto Status = FS.status(WPath);
-    if (Status && Status->getType() == llvm::sys::fs::file_type::regular_file) {
-      FilePath = std::move(WPath);
-      return true;
-    }
-  }
-  return false;
-}
-
 static void appendOneArg(InputArgList &Args, const Arg *Opt,
                          const Arg *BaseArg) {
   // The args for config files or /clang: flags belong to different InputArgList
   // objects than Args. This copies an Arg from one of those other InputArgLists
   // to the ownership of Args.
   unsigned Index = Args.MakeIndex(Opt->getSpelling());
   Arg *Copy = new llvm::opt::Arg(Opt->getOption(), Args.getArgString(Index),
                                  Index, BaseArg);
   Copy->getValues() = Opt->getValues();
   if (Opt->isClaimed())
     Copy->claim();
   Copy->setOwnsValues(Opt->getOwnsValues());
   Opt->setOwnsValues(false);
   Args.append(Copy);
 }
 
-bool Driver::readConfigFile(StringRef FileName) {
+bool Driver::readConfigFile(StringRef FileName,
+                            llvm::cl::ExpansionContext &ExpCtx) {
   // Try reading the given file.
   SmallVector<const char *, 32> NewCfgArgs;
-  llvm::cl::ExpansionContext ExpCtx(Alloc, llvm::cl::tokenizeConfigFile);
-  ExpCtx.setVFS(&getVFS());
   if (!ExpCtx.readConfigFile(FileName, NewCfgArgs)) {
     Diag(diag::err_drv_cannot_read_config_file) << FileName;
     return true;
   }
 
   // Read options from config file.
   llvm::SmallString<128> CfgFileName(FileName);
   llvm::sys::path::native(CfgFileName);
for (auto *Opt : *NewOptions) {
       appendOneArg(*CfgOptions, Opt, BaseArg);
     }
   }
   ConfigFiles.push_back(std::string(CfgFileName));
   return false;
 }
 
 bool Driver::loadConfigFiles() {
+  llvm::cl::ExpansionContext ExpCtx(Saver.getAllocator(),
+                                    llvm::cl::tokenizeConfigFile);
+  ExpCtx.setVFS(&getVFS());
+
   // Process options that change search path for config files.
   if (CLOptions) {
     if (CLOptions->hasArg(options::OPT_config_system_dir_EQ)) {
       SmallString<128> CfgDir;
       CfgDir.append(
           CLOptions->getLastArgValue(options::OPT_config_system_dir_EQ));
       if (CfgDir.empty() || getVFS().makeAbsolute(CfgDir))
         SystemConfigDir.clear();
       else
         SystemConfigDir = static_cast<std::string>(CfgDir);
     }
     if (CLOptions->hasArg(options::OPT_config_user_dir_EQ)) {
       SmallString<128> CfgDir;
       CfgDir.append(
           CLOptions->getLastArgValue(options::OPT_config_user_dir_EQ));
       if (CfgDir.empty() || getVFS().makeAbsolute(CfgDir))
         UserConfigDir.clear();
       else
         UserConfigDir = static_cast<std::string>(CfgDir);
     }
   }
 
   // Prepare list of directories where config file is searched for.
   StringRef CfgFileSearchDirs[] = {UserConfigDir, SystemConfigDir, Dir};
+  ExpCtx.setSearchDirs(CfgFileSearchDirs);
 
   // First try to load configuration from the default files, return on error.
-  if (loadDefaultConfigFiles(CfgFileSearchDirs))
+  if (loadDefaultConfigFiles(ExpCtx))
     return true;
 
   // Then load configuration files specified explicitly.
-  llvm::SmallString<128> CfgFilePath;
+  SmallString<128> CfgFilePath;
   if (CLOptions) {
     for (auto CfgFileName : CLOptions->getAllArgValues(options::OPT_config)) {
       // If argument contains directory separator, treat it as a path to
       // configuration file.
       if (llvm::sys::path::has_parent_path(CfgFileName)) {
-        CfgFilePath = CfgFileName;
+        CfgFilePath.assign(CfgFileName);
         if (llvm::sys::path::is_relative(CfgFilePath)) {
           if (getVFS().makeAbsolute(CfgFilePath))
             return true;
           auto Status = getVFS().status(CfgFilePath);
           if (!Status ||
               Status->getType() != llvm::sys::fs::file_type::regular_file) {
             Diag(diag::err_drv_config_file_not_exist) << CfgFilePath;
             return true;
           }
         }
-      } else if (!searchForFile(CfgFilePath, CfgFileSearchDirs, CfgFileName,
-                                getVFS())) {
+      } else if (!ExpCtx.findConfigFile(CfgFileName, CfgFilePath)) {
         // Report an error that the config file could not be found.
         Diag(diag::err_drv_config_file_not_found) << CfgFileName;
         for (const StringRef &SearchDir : CfgFileSearchDirs)
           if (!SearchDir.empty())
             Diag(diag::note_drv_config_file_searched_in) << SearchDir;
         return true;
       }
 
       // Try to read the config file, return on error.
-      if (readConfigFile(CfgFilePath))
+      if (readConfigFile(CfgFilePath, ExpCtx))
         return true;
     }
   }
 
   // No error occurred.
   return false;
 }
 
-bool Driver::loadDefaultConfigFiles(ArrayRef<StringRef> CfgFileSearchDirs) {
+bool Driver::loadDefaultConfigFiles(llvm::cl::ExpansionContext &ExpCtx) {
   // Disable default config if CLANG_NO_DEFAULT_CONFIG is set to a non-empty
   // value.
   if (const char *NoConfigEnv = ::getenv("CLANG_NO_DEFAULT_CONFIG")) {
     if (*NoConfigEnv)
       return false;
   }
   if (CLOptions && CLOptions->hasArg(options::OPT_no_default_config))
     return false;
bool Driver::loadDefaultConfigFiles(llvm::cl::ExpansionContext &ExpCtx) {
   // 2. <triple>-<mode>.cfg using executable suffix
   //    (e.g. i386-pc-linux-gnu-clang-g++.cfg for *clang-g++).
   // 3. <triple>.cfg + <mode>.cfg using real driver mode
   //    (e.g. i386-pc-linux-gnu.cfg + clang++.cfg).
   // 4. <triple>.cfg + <mode>.cfg using executable suffix
   //    (e.g. i386-pc-linux-gnu.cfg + clang-g++.cfg for *clang-g++).
 
   // Try loading <triple>-<mode>.cfg, and return if we find a match.
-  llvm::SmallString<128> CfgFilePath;
+  SmallString<128> CfgFilePath;
   std::string CfgFileName = Triple + '-' + RealMode + ".cfg";
-  if (searchForFile(CfgFilePath, CfgFileSearchDirs, CfgFileName, getVFS()))
-    return readConfigFile(CfgFilePath);
+  if (ExpCtx.findConfigFile(CfgFileName, CfgFilePath))
+    return readConfigFile(CfgFilePath, ExpCtx);
 
   bool TryModeSuffix = !ClangNameParts.ModeSuffix.empty() &&
                        ClangNameParts.ModeSuffix != RealMode;
   if (TryModeSuffix) {
     CfgFileName = Triple + '-' + ClangNameParts.ModeSuffix + ".cfg";
-    if (searchForFile(CfgFilePath, CfgFileSearchDirs, CfgFileName, getVFS()))
-      return readConfigFile(CfgFilePath);
+    if (ExpCtx.findConfigFile(CfgFileName, CfgFilePath))
+      return readConfigFile(CfgFilePath, ExpCtx);
   }
 
   // Try loading <mode>.cfg, and return if loading failed.  If a matching file
   // was not found, still proceed on to try <triple>.cfg.
   CfgFileName = RealMode + ".cfg";
-  if (searchForFile(CfgFilePath, CfgFileSearchDirs, CfgFileName, getVFS())) {
-    if (readConfigFile(CfgFilePath))
+  if (ExpCtx.findConfigFile(CfgFileName, CfgFilePath)) {
+    if (readConfigFile(CfgFilePath, ExpCtx))
       return true;
   } else if (TryModeSuffix) {
     CfgFileName = ClangNameParts.ModeSuffix + ".cfg";
-    if (searchForFile(CfgFilePath, CfgFileSearchDirs, CfgFileName, getVFS()) &&
-        readConfigFile(CfgFilePath))
+    if (ExpCtx.findConfigFile(CfgFileName, CfgFilePath) &&
+        readConfigFile(CfgFilePath, ExpCtx))
       return true;
   }
 
   // Try loading <triple>.cfg and return if we find a match.
   CfgFileName = Triple + ".cfg";
-  if (searchForFile(CfgFilePath, CfgFileSearchDirs, CfgFileName, getVFS()))
-    return readConfigFile(CfgFilePath);
+  if (ExpCtx.findConfigFile(CfgFileName, CfgFilePath))
+    return readConfigFile(CfgFilePath, ExpCtx);
 
   // If we were unable to find a config file deduced from executable name,
   // that is not an error.
   return false;
 }
 
 Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
   llvm::PrettyStackTraceString CrashInfo("Compilation construction");

llvm/include/llvm/Support/CommandLine.h

class ExpansionContext {
 
   /// File system used for all file access when running the expansion.
   vfs::FileSystem *FS;
 
   /// Path used to resolve relative rsp files. If empty, the file system
   /// current directory is used instead.
   StringRef CurrentDir;
 
+  /// Directories used for search of config files.
+  ArrayRef<StringRef> SearchDirs;
+
   /// True if names of nested response files must be resolved relative to
   /// including file.
   bool RelativeNames = false;
 
   /// If true, mark end of lines and the end of the response file with nullptrs
   /// in the Argv vector.
   bool MarkEOLs = false;
 
ExpansionContext &setRelativeNames(bool X) {
     return *this;
   }
 
   ExpansionContext &setCurrentDir(StringRef X) {
     CurrentDir = X;
     return *this;
   }
 
+  ExpansionContext &setSearchDirs(ArrayRef<StringRef> X) {
+    SearchDirs = X;
+    return *this;
+  }
+
   ExpansionContext &setVFS(vfs::FileSystem *X) {
     FS = X;
     return *this;
   }
 
+  /// Looks for the specified configuration file.
+  ///
+  /// \param[in]  FileName Name of the file to search for.
+  /// \param[out] FilePath File absolute path, if it was found.
+  /// \return True if file was found.
+  ///
+  /// If the specified file name contains a directory separator, it is searched
+  /// for by its absolute path. Otherwise looks for file sequentially in
+  /// directories specified by SearchDirs field.
+  bool findConfigFile(StringRef FileName, SmallVectorImpl<char> &FilePath);
+
   /// Reads command line options from the given configuration file.
   ///
   /// \param [in] CfgFile Path to configuration file.
   /// \param [out] Argv Array to which the read options are added.
   /// \return true if the file was successfully read.
   ///
   /// It reads content of the specified file, tokenizes it and expands "@file"
   /// commands resolving file names in them relative to the directory where

llvm/lib/Support/CommandLine.cpp

bool cl::expandResponseFiles(int Argc, const char *const *Argv,
   NewArgv.append(Argv + 1, Argv + Argc);
   ExpansionContext ECtx(Saver.getAllocator(), Tokenize);
   return ECtx.expandResponseFiles(NewArgv);
 }
 
 ExpansionContext::ExpansionContext(BumpPtrAllocator &A, TokenizerCallback T)
     : Saver(A), Tokenizer(T), FS(vfs::getRealFileSystem().get()) {}
 
+bool ExpansionContext::findConfigFile(StringRef FileName,
+                                      SmallVectorImpl<char> &FilePath) {
+  SmallString<128> CfgFilePath;
+  const auto FileExists = [this](SmallString<128> Path) -> bool {
+    auto Status = FS->status(Path);
+    return Status &&
+           Status->getType() == llvm::sys::fs::file_type::regular_file;
+  };
+
+  // If file name contains directory separator, treat it as a path to
+  // configuration file.
+  if (llvm::sys::path::has_parent_path(FileName)) {
+    CfgFilePath = FileName;
+    if (llvm::sys::path::is_relative(FileName) && FS->makeAbsolute(CfgFilePath))
+      return false;
+    if (!FileExists(CfgFilePath))
+      return false;
+    FilePath.assign(CfgFilePath.begin(), CfgFilePath.end());
+    return true;
+  }
+
+  // Look for the file in search directories.
+  for (const StringRef &Dir : SearchDirs) {
+    if (Dir.empty())
+      continue;
+    CfgFilePath.assign(Dir);
+    llvm::sys::path::append(CfgFilePath, FileName);
+    llvm::sys::path::native(CfgFilePath);
+    if (FileExists(CfgFilePath)) {
+      FilePath.assign(CfgFilePath.begin(), CfgFilePath.end());
+      return true;
+    }
+  }
+
+  return false;
+}
+
 bool ExpansionContext::readConfigFile(StringRef CfgFile,
                                       SmallVectorImpl<const char *> &Argv) {
   SmallString<128> AbsPath;
   if (sys::path::is_relative(CfgFile)) {
     AbsPath.assign(CfgFile);
     if (std::error_code EC = FS->makeAbsolute(AbsPath))
       return false;
     CfgFile = AbsPath.str();