[PATCH] D24933: Enable configuration files in clang

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.


================
Comment at: lib/Driver/Driver.cpp:172
+    NumConfigArgs = static_cast<unsigned>(NumCfgArgs);
+  }
+
----------------
sepavloff wrote:
> bruno wrote:
> > If `NumCfgArgs == 0` it would be nice to warn that the config file is empty?
> Not sure if it makes sense. Usually warning are used to attract attention to the things that potentially may be harmful. An empty config file is strange but does not look dangerous. 
I agree. The config files might be automatically generated by wrapper scripts, and adding special cases to avoid creating empty files seems like an unnecessary complication.


================
Comment at: lib/Driver/Driver.cpp:2695
 
+  // Clain all arguments that come from a configuration file so that the driver
+  // does not warn on any that are unused.
----------------
Clain -> Claim


================
Comment at: tools/driver/driver.cpp:332
+  StringRef PName = llvm::sys::path::stem(ProgramName);
+  size_t Pos = PName.find("-clang");
+  if (Pos == StringRef::npos)
----------------
Looking for "-clang" is too specific; our driver-name suffix processing allows more general naming than this. For one thing, it will not pick up armv7l-cpp correctly (which would be a problem because the config files likely contain options affecting preprocessing). I also have users which use symlinks to clang named fooclang.

I think that an easy way to do this is to use the result from ToolChain::getTargetAndModeFromProgramName, which we call from the caller of this function:

  std::string ProgName = argv[0];
  std::pair<std::string, std::string> TargetAndMode =
      ToolChain::getTargetAndModeFromProgramName(ProgName);

We can use TargetAndMode.first here, instead of looking for the string before "-clang", and that should be consistent with the rest of our processing.



https://reviews.llvm.org/D24933