[PATCH] clang-cl: Sink /Fe and /Fo diagnostic code into BuildActions

[Hans Wennborg]

Hi rnk,

This sinks the diagnostic code into BuildActions, which seems better because there's already some diagnostics stuff there.

The annoying thing is that the code needs to modify the ArgList, so the ArgList parameter to BuildActions can't be const anymore :/ Not sure if that's a big deal, though.

http://llvm-reviews.chandlerc.com/D1370

Files:
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp

Index: include/clang/Driver/Driver.h
===================================================================
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -266,7 +266,7 @@
   /// \param TC - The default host tool chain.
   /// \param Args - The input arguments.
   /// \param Actions - The list to store the resulting actions onto.
-  void BuildActions(const ToolChain &TC, const llvm::opt::DerivedArgList &Args,
+  void BuildActions(const ToolChain &TC, llvm::opt::DerivedArgList &Args,
                     const InputList &Inputs, ActionList &Actions) const;
 
   /// BuildUniversalActions - Construct the list of actions to perform
@@ -276,7 +276,7 @@
   /// \param Args - The input arguments.
   /// \param Actions - The list to store the resulting actions onto.
   void BuildUniversalActions(const ToolChain &TC,
-                             const llvm::opt::DerivedArgList &Args,
+                             llvm::opt::DerivedArgList &Args,
                              const InputList &BAInputs,
                              ActionList &Actions) const;
 
Index: lib/Driver/Driver.cpp
===================================================================
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -377,31 +377,6 @@
   InputList Inputs;
   BuildInputs(C->getDefaultToolChain(), C->getArgs(), Inputs);
 
-  if (Arg *A = C->getArgs().getLastArg(options::OPT__SLASH_Fo)) {
-    DiagnoseOptionOverride(*this, C->getArgs(), options::OPT__SLASH_Fo);
-    StringRef V = A->getValue();
-    if (V.empty()) {
-      // It has to have a value.
-      Diag(clang::diag::err_drv_missing_argument) << A->getSpelling() << 1;
-      C->getArgs().eraseArg(options::OPT__SLASH_Fo);
-    } else if (Inputs.size() > 1 && !llvm::sys::path::is_separator(V.back())) {
-      // Check whether /Fo tries to name an output file for multiple inputs.
-      Diag(clang::diag::err_drv_obj_file_argument_with_multiple_sources)
-        << A->getSpelling() << V;
-      C->getArgs().eraseArg(options::OPT__SLASH_Fo);
-    }
-  }
-
-  if (Arg *A = C->getArgs().getLastArg(options::OPT__SLASH_Fe)) {
-    DiagnoseOptionOverride(*this, C->getArgs(), options::OPT__SLASH_Fe);
-
-    if (A->getValue()[0] == '\0') {
-      // It has to have a value.
-      Diag(clang::diag::err_drv_missing_argument) << A->getSpelling() << 1;
-      C->getArgs().eraseArg(options::OPT__SLASH_Fe);
-    }
-  }
-
   // Construct the list of abstract actions to perform for this compilation. On
   // Darwin target OSes this uses the driver-driver and universal actions.
   if (TC.getTriple().isOSDarwin())
@@ -893,7 +868,7 @@
 }
 
 void Driver::BuildUniversalActions(const ToolChain &TC,
-                                   const DerivedArgList &Args,
+                                   DerivedArgList &Args,
                                    const InputList &BAInputs,
                                    ActionList &Actions) const {
   llvm::PrettyStackTraceString CrashInfo("Building universal build actions");
@@ -1169,7 +1144,7 @@
   }
 }
 
-void Driver::BuildActions(const ToolChain &TC, const DerivedArgList &Args,
+void Driver::BuildActions(const ToolChain &TC, DerivedArgList &Args,
                           const InputList &Inputs, ActionList &Actions) const {
   llvm::PrettyStackTraceString CrashInfo("Building compilation actions");
 
@@ -1186,6 +1161,32 @@
   if (Arg *A = Args.getLastArg(options::OPT_Z_Joined))
     Diag(clang::diag::err_drv_use_of_Z_option) << A->getAsString(Args);
 
+  // Diagnose misuse of /Fo.
+  if (Arg *A = Args.getLastArg(options::OPT__SLASH_Fo)) {
+    DiagnoseOptionOverride(*this, Args, options::OPT__SLASH_Fo);
+    StringRef V = A->getValue();
+    if (V.empty()) {
+      // It has to have a value.
+      Diag(clang::diag::err_drv_missing_argument) << A->getSpelling() << 1;
+      Args.eraseArg(options::OPT__SLASH_Fo);
+    } else if (Inputs.size() > 1 && !llvm::sys::path::is_separator(V.back())) {
+      // Check whether /Fo tries to name an output file for multiple inputs.
+      Diag(clang::diag::err_drv_obj_file_argument_with_multiple_sources)
+        << A->getSpelling() << V;
+      Args.eraseArg(options::OPT__SLASH_Fo);
+    }
+  }
+
+  // Diagnose misuse of /Fe.
+  if (Arg *A = Args.getLastArg(options::OPT__SLASH_Fe)) {
+    DiagnoseOptionOverride(*this, Args, options::OPT__SLASH_Fe);
+    if (A->getValue()[0] == '\0') {
+      // It has to have a value.
+      Diag(clang::diag::err_drv_missing_argument) << A->getSpelling() << 1;
+      Args.eraseArg(options::OPT__SLASH_Fe);
+    }
+  }
+
   // Construct the actions to perform.
   ActionList LinkerInputs;
   ActionList SplitInputs;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1370.1.patch
Type: text/x-patch
Size: 4651 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130812/f6131680/attachment.bin>