[PATCH] clang-cl: Don't warn about overriding /MD with /MT, /Fo with another /Fo, etc.

[Hans Wennborg]

Hi rnk,

I'm interested to hear your thoughts on this.

I put in the warnings because MSVC has them, but I don't think they're very useful.

Clang does not warn about overriding flags in general, e.g. it's perfectly fine to have -fomit-frame-pointer followed by -fno-omit-frame-pointer.

We should focus on warning where things get confusing, such as with the /TP and /TC options. In "clang-cl /TC a.c /TP b.cc", the user might not realize that the /TP flag will apply to both files, and we warn about that.

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

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/Driver.cpp
  lib/Driver/Tools.cpp
  test/Driver/cl-outputs.c
  test/Driver/cl-runtime-flags.c

Index: include/clang/Basic/DiagnosticDriverKinds.td
===================================================================
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -138,9 +138,6 @@
   "unknown platform, assuming -mfloat-abi=%0">;
 def warn_ignoring_ftabstop_value : Warning<
   "ignoring invalid -ftabstop value '%0', using default value %1">;
-def warn_drv_overriding_joined_option : Warning<
-  "overriding '%0%1' option with '%2%3'">,
-  InGroup<DiagGroup<"overriding-fo-option">>;
 def warn_drv_overriding_flag_option : Warning<
   "overriding '%0' option with '%1'">,
   InGroup<DiagGroup<"overriding-t-option">>;
Index: lib/Driver/Driver.cpp
===================================================================
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -284,26 +284,6 @@
   return DAL;
 }
 
-/// \brief Check whether there are multiple instances of OptionID in Args, and
-/// if so, issue a diagnostics about it.
-static void DiagnoseOptionOverride(const Driver &D, const DerivedArgList &Args,
-                                   unsigned OptionID) {
-  assert(Args.hasArg(OptionID));
-
-  arg_iterator it = Args.filtered_begin(OptionID);
-  arg_iterator ie = Args.filtered_end();
-  Arg *Previous = *it;
-  ++it;
-
-  while (it != ie) {
-    D.Diag(clang::diag::warn_drv_overriding_joined_option)
-        << Previous->getSpelling() << Previous->getValue()
-        << (*it)->getSpelling() << (*it)->getValue();
-    Previous = *it;
-    ++it;
-  }
-}
-
 Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
   llvm::PrettyStackTraceString CrashInfo("Compilation construction");
 
@@ -1158,7 +1138,6 @@
 
   // 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.
@@ -1174,7 +1153,6 @@
 
   // 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;
Index: lib/Driver/Tools.cpp
===================================================================
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3724,21 +3724,9 @@
     // but defining _DEBUG is sticky.
     RTOptionID = options::OPT__SLASH_MTd;
 
-  if (Arg *A = Args.getLastArg(options::OPT__SLASH_M_Group)) {
+  if (Arg *A = Args.getLastArg(options::OPT__SLASH_M_Group))
     RTOptionID = A->getOption().getID();
 
-    // Diagnose overrides.
-    arg_iterator it = Args.filtered_begin(options::OPT__SLASH_M_Group);
-    Arg *Previous = *it++;
-    const arg_iterator ie = Args.filtered_end();
-    while (it != ie) {
-      const Driver &D = getToolChain().getDriver();
-      D.Diag(clang::diag::warn_drv_overriding_flag_option)
-        << Previous->getSpelling() << (*it)->getSpelling();
-      Previous = *it++;
-    }
-  }
-
   switch(RTOptionID) {
     case options::OPT__SLASH_MD:
       if (Args.hasArg(options::OPT__SLASH_LDd))
Index: test/Driver/cl-outputs.c
===================================================================
--- test/Driver/cl-outputs.c
+++ test/Driver/cl-outputs.c
@@ -11,7 +11,6 @@
 // FoNAME:  "-o" "a.obj"
 
 // RUN: %clang_cl /Foa.ext /Fob.ext -### -- %s 2>&1 | FileCheck -check-prefix=FoNAMEEXT %s
-// FoNAMEEXT:  warning: overriding '/Foa.ext' option with '/Fob.ext'
 // FoNAMEEXT:  "-o" "b.ext"
 
 // RUN: %clang_cl /Fofoo.dir/ -### -- %s 2>&1 | FileCheck -check-prefix=FoDIR %s
@@ -88,5 +87,4 @@
 // FeMISSINGARG: error: argument to '/Fe' is missing (expected 1 value)
 
 // RUN: %clang_cl /Fefoo /Febar -### -- %s 2>&1 | FileCheck -check-prefix=FeOVERRIDE %s
-// FeOVERRIDE: warning: overriding '/Fefoo' option with '/Febar'
 // FeOVERRIDE: "-out:bar.exe"
Index: test/Driver/cl-runtime-flags.c
===================================================================
--- test/Driver/cl-runtime-flags.c
+++ test/Driver/cl-runtime-flags.c
@@ -86,5 +86,4 @@
 // CHECK-LDMDd: "--dependent-lib=msvcrtd"
 
 // RUN: %clang_cl /MD /MT -### -- %s 2>&1 | FileCheck -check-prefix=MTOVERRIDE %s
-// MTOVERRIDE: warning: overriding '/MD' option with '/MT'
 // MTOVERRIDE: "--dependent-lib=libcmt"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1718.1.patch
Type: text/x-patch
Size: 4399 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130918/f043bf86/attachment.bin>