[Driver] Fix erroneous warning for -fcx-limited-range and -fcx-fortran-rules.

[zahiraam]

The options -fcx-limited-range and -fcx-fortran-rules were added in #70244

The code adding the options introduced an erroneous warning.
$ clang -c -fcx-limited-range t1.c
clang: warning: overriding '' option with '-fcx-limited-range' [-Woverriding-option]
and
$ clang -c -fcx-fortran-rules t1.c
clang: warning: overriding '' option with '-fcx-fortran-rules' [-Woverriding-option]

The warning doesn't make sense. This patch removes it.

[llvmbot]

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Zahira Ammarguellat (zahiraam)

Changes

This patch is to remove erroneous warning. See https://godbolt.org/z/bYP8P8nqY

---

Full diff: https://github.com/llvm/llvm-project/pull/79821.diff

2 Files Affected:

• (modified) clang/lib/Driver/ToolChains/Clang.cpp (+1-1)
• (modified) clang/test/Driver/range.c (+4)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 8d8965fdf76fb8a..7ceb248f7afb99e 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2705,7 +2705,7 @@ static StringRef EnumComplexRangeToStr(LangOptions::ComplexRangeKind Range) {
 static void EmitComplexRangeDiag(const Driver &D,
                                  LangOptions::ComplexRangeKind Range1,
                                  LangOptions::ComplexRangeKind Range2) {
-  if (Range1 != LangOptions::ComplexRangeKind::CX_Full)
+  if (Range1 != Range2 && Range1 != LangOptions::ComplexRangeKind::CX_None)
     D.Diag(clang::diag::warn_drv_overriding_option)
         << EnumComplexRangeToStr(Range1) << EnumComplexRangeToStr(Range2);
 }
diff --git a/clang/test/Driver/range.c b/clang/test/Driver/range.c
index 045d9c7d3d802ae..023b5cffbe3e0e4 100644
--- a/clang/test/Driver/range.c
+++ b/clang/test/Driver/range.c
@@ -35,6 +35,9 @@
 // RUN: %clang -### -target x86_64 -ffast-math -fno-cx-limited-range -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=FULL %s
 
+// RUN: %clang -c -target x86_64 -fcx-limited-range %s -Xclang -verify=range
+// RUN: %clang -c -target x86_64 -fcx-fortran-rules %s -Xclang -verify=range
+
 // LMTD: -complex-range=limited
 // FULL: -complex-range=full
 // LMTD-NOT: -complex-range=fortran
@@ -44,3 +47,4 @@
 // CHECK-NOT: -complex-range=fortran
 // WARN1: warning: overriding '-fcx-limited-range' option with '-fcx-fortran-rules' [-Woverriding-option]
 // WARN2: warning: overriding '-fcx-fortran-rules' option with '-fcx-limited-range' [-Woverriding-option]
+// range-no-diagnostics

[AaronBallman]

Thank you for the fix! Can you add some more description to the patch summary as to what's erroneous (the summary ends up becoming the commit message, so a terse summary can be harmful for later code archeology).

Do we need a release note for this? Are you planning to cherry-pick this to the 18.x branch?

[AaronBallman]

CC @MaskRay is this a reasonable way to test or should we be using CHECK-NOT and not using -verify because the diagnostics come from the driver?

[MaskRay]

-verify does not work for driver errors/warnings. You can add -Werror. You omit -###, so this would run code generation, which is typically not desired for test/Driver tests.

In case of errors, clang -### will exit with code 1. Since lit requires that commands exit with code 0, -### -Werror alone serves as a test.

% clang -### -c -fcx-fortran-rules a.c -Werror
...
clang: error: overriding '' option with '-fcx-fortran-rules' [-Werror,-Woverriding-option]
...

[zahiraam]

@MaskRay Thanks. Let me know if the test is correct now.

[MaskRay]

Delete the unused // range-no-diagnostics

[zahiraam]

Thank you for the fix! Can you add some more description to the patch summary as to what's erroneous (the summary ends up becoming the commit message, so a terse summary can be harmful for later code archeology).

Do we need a release note for this? Are you planning to cherry-pick this to the 18.x branch?

I don't think we need a Release note for it since this was a faulty behavior to start with. It's not related to any of the options.

Would be nice to cherry-pick. Not sure how to do that though.

[AaronBallman]

I don't think we need a Release note for it since this was a faulty behavior to start with. It's not related to any of the options.

Ah, good to know, thanks!

Would be nice to cherry-pick. Not sure how to do that though.

https://llvm.org/docs/GitHub.html#backporting-fixes-to-the-release-branches

[MaskRay]

Fix erroneous warning.

I'd suggest a more descriptive title, e.g. [Driver] Fix erroneous warning for -fcx-limited-range and -fcx-fortran-rules

Please also mention #70244 in the description. I am not familiar with the two options, I'll trust your judgement as the author of #70244 :)

[MaskRay]

.

[MaskRay]

Delete the unused // range-no-diagnostics

[zahiraam]

@AaronBallman Any more comments?

[AaronBallman]

LGTM, thank you for the fix!

[zahiraam]

@MaskRay, @AaronBallman thanks for the review.