[PATCH] D24933: Enable configuration files in clang

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.


================
Comment at: include/clang/Driver/Driver.h:219
+  /// This number may be smaller that \c NumConfigOptions if some options
+  /// requires separate arguments.
+  unsigned NumConfigArgs;
----------------
requires -> require


================
Comment at: lib/Driver/ToolChain.cpp:183
   std::string Target;
-  if (llvm::TargetRegistry::lookupTarget(Prefix, IgnoredError)) {
+  if (!VerifyTarget || llvm::TargetRegistry::lookupTarget(Prefix, IgnoredError))
     Target = Prefix;
----------------
I don't think that we can do it this way; it is a behavior change (we now might try to set the target to some string which did not validate as a known target, whereas we did not previously).

How about you always return the prefix, but also return a boolean indicating whether or not the prefix is a valid target? Then, after processing the config file, you can clear out the string if it was not a valid target.


================
Comment at: tools/driver/driver.cpp:363
+                                       TargetAndMode.first + ".cfg");
+    TargetAndMode.first.clear();
+  }
----------------
I don't think that you can clear the string here. We might need it later to call insertTargetAndModeArgs.


================
Comment at: tools/driver/driver.cpp:449
+      // there are options read from config file, put the options from "CL"
+      // after them, - config file is considered as a "patch" to compiler
+      // defaults.
----------------
Replace
  after them, - config file
with
  after them because the config file



https://reviews.llvm.org/D24933