[darwin][driver] Warn about mismatching -<os>-version-min rather than superfluous -<os>-version-min compiler option

[arphaman]

Summary

The warning about the superfluous -<os>-version-min compiler option seems a little too strong right now. For now we should only warn about -<os>-version-min options that specify a different OS version to the -target's version.

[bob.wilson]

Eventually it would be nice to also warn about redundant -m*-version-min options, but for now I agree that it would be best to start with warning only when the options are different.

[steven_wu]

Just a small suggestion. Looks good otherwise.

lib/Driver/ToolChains/Darwin.cpp

1536

HadExtra is not ok right? macos10.11.0.1 is not the same as macos10.11.0?
Or HadExtra is an error somewhere else already?

[arphaman]

lib/Driver/ToolChains/Darwin.cpp

1536

Right, it would be nice to check the extra stuff too. I will commit with the check and a test for the extra part.

[steven_wu]

lib/Driver/ToolChains/Darwin.cpp

1536

And what about "-target x86_64-apple-macos -mmacos-version-min=10.6"?

[arphaman]

lib/Driver/ToolChains/Darwin.cpp

1536

We will warn on that.

[MaskRay]

cfe/trunk/lib/Driver/ToolChains/Darwin.cpp

1545 ↗

(On Diff #127648)

warn_drv_overriding_flag_option is in the group -Woverriding-t-option (D1290), which is for clang-cl /T* options.
I think we need a different diagnostic group for this purpose. I created D158137 to add warn_drv_overriding_option.

lib/Driver/ToolChains/Darwin.cpp

if (char *env = ::getenv("SDKROOT")) {
       }
     }
   }
 
   // The OS and the version can be specified using the -target argument.
   Optional<DarwinPlatform> OSTarget =
       getDeploymentTargetFromTargetArg(Args, getTriple(), getDriver());
   if (OSTarget) {
-    // Warn about superfluous -m<os>-version-min arg.
     Optional<DarwinPlatform> OSVersionArgTarget =
         getDeploymentTargetFromOSVersionArg(Args, getDriver());
-    if (OSVersionArgTarget)
-      getDriver().Diag(clang::diag::warn_drv_unused_argument)
-          << OSVersionArgTarget->getAsString(Args, Opts);
+    if (OSVersionArgTarget) {
+      unsigned TargetMajor, TargetMinor, TargetMicro;
+      unsigned ArgMajor, ArgMinor, ArgMicro;
+      bool HadExtra;
+      if (OSTarget->getPlatform() != OSVersionArgTarget->getPlatform() ||
+          (Driver::GetReleaseVersion(OSTarget->getOSVersion(), TargetMajor,
+                                     TargetMinor, TargetMicro, HadExtra) &&
+           Driver::GetReleaseVersion(OSVersionArgTarget->getOSVersion(),
+                                     ArgMajor, ArgMinor, ArgMicro, HadExtra) &&
+           VersionTuple(TargetMajor, TargetMinor, TargetMicro) !=
+               VersionTuple(ArgMajor, ArgMinor, ArgMicro))) {
+        // Warn about -m<os>-version-min that doesn't match the OS version
+        // that's specified in the target.
+        std::string OSVersionArg = OSVersionArgTarget->getAsString(Args, Opts);
+        std::string TargetArg = OSTarget->getAsString(Args, Opts);
+        getDriver().Diag(clang::diag::warn_drv_overriding_flag_option)
+            << OSVersionArg << TargetArg;
+      }
+    }
   } else {
     // The OS target can be specified using the -m<os>version-min argument.
     OSTarget = getDeploymentTargetFromOSVersionArg(Args, getDriver());
     // If no deployment target was specified on the command line, check for
     // environment defines.
     if (!OSTarget)
       OSTarget =
           getDeploymentTargetFromEnvironmentVariables(getDriver(), getTriple());

test/Driver/darwin-version.c

 // RUN: %clang -target unknown-apple-darwin12  -arch armv7  -c %s -### 2>&1 | \
 // RUN:   FileCheck --check-prefix=CHECK-VERSION-TDARWIN-FALL5 %s
 // CHECK-VERSION-TDARWIN-FALL5: "thumbv7-apple-ios5.0.0"
 
 // Warn about -m<os>-version-min when it's used with target:
 
 // RUN: %clang -target x86_64-apple-macos10.11.2 -mmacos-version-min=10.6 -c %s -### 2>&1 | \
 // RUN:   FileCheck --check-prefix=CHECK-VERSION-TNO-OSV1 %s
-// CHECK-VERSION-TNO-OSV1: argument unused during compilation: '-mmacosx-version-min=10.6'
+// CHECK-VERSION-TNO-OSV1: overriding '-mmacosx-version-min=10.6' option with '--target=x86_64-apple-macos10.11.2'
 
 // RUN: %clang -target x86_64-apple-macos -miphoneos-version-min=9.1 -c %s -### 2>&1 | \
 // RUN:   FileCheck --check-prefix=CHECK-VERSION-TNO-OSV2 %s
-// CHECK-VERSION-TNO-OSV2: argument unused during compilation: '-miphoneos-version-min=9.1'
+// CHECK-VERSION-TNO-OSV2: overriding '-miphoneos-version-min=9.1' option with '--target=x86_64-apple-macos'
+
+// RUN: %clang -target x86_64-apple-ios -miphonesimulator-version-min=10.0 -c %s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=CHECK-VERSION-TNO-OSV3 %s
+// CHECK-VERSION-TNO-OSV3: overriding '-mios-simulator-version-min=10.0' option with '--target=x86_64-apple-ios'
+// CHECK-VERSION-TNO-OSV3-NOT: argument unused during compilation
+
+// RUN: %clang -target x86_64-apple-macos10.6 -mmacos-version-min=10.6 -c %s -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=CHECK-VERSION-TNO-SAME %s
+// CHECK-VERSION-TNO-SAME-NOT: overriding
+// CHECK-VERSION-TNO-SAME-NOT: argument unused during compilation
 
 // Target with OS version is not overriden by -m<os>-version-min variables:
 
 // RUN: %clang -target x86_64-apple-macos10.11.2 -mmacos-version-min=10.6 -c %s -### 2>&1 | \
 // RUN:   FileCheck --check-prefix=CHECK-VERSION-TIGNORE-OSV1 %s
 // CHECK-VERSION-TIGNORE-OSV1: "x86_64-apple-macosx10.11.2"
 
 // RUN: %clang -target arm64-apple-ios11.0 -mios-version-min=9.0 -c %s -### 2>&1 | \