Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix warning message when using negative complex range options. #84567

Merged
merged 2 commits into from
Mar 12, 2024

Conversation

zahiraam
Copy link
Contributor

@zahiraam zahiraam commented Mar 8, 2024

When -fcx-no-limited-range or -fno-cx-fortran-rules follows another complex range option on the command line, it will trigger a warning with empty message.
warning: overriding '-fcx-fortran-rules' option with '' [-Woverriding-option]
or
warning: overriding '-fcx-limited-range' option with '' [-Woverriding-option]
This patch fixes that.

@zahiraam zahiraam changed the title Fix warning message when using -fno-cx-limited-range and -fno-cx-fort… Fix warning message when using -fno-cx-limited-range/cx-fort… Mar 8, 2024
@zahiraam zahiraam changed the title Fix warning message when using -fno-cx-limited-range/cx-fort… Fix warning message when using negative complex range options. Mar 8, 2024
@zahiraam zahiraam marked this pull request as ready for review March 8, 2024 21:21
@zahiraam zahiraam requested a review from MaskRay March 8, 2024 21:21
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Mar 8, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 8, 2024

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Zahira Ammarguellat (zahiraam)

Changes

When -fcx-no-limited-range or -fno-cx-fortran-rules follows another complex range option on the command line, it will trigger a warning with empty message.
warning: overriding '-fcx-fortran-rules' option with '' [-Woverriding-option]
or
warning: overriding '-fcx-limited-range' option with '' [-Woverriding-option]
This patch fixes that.


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

2 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+26-9)
  • (modified) clang/test/Driver/range.c (+18-5)
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 858d20fbfac015..46faf467dec0de 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2687,8 +2687,8 @@ static void CollectArgsForIntegratedAssembler(Compilation &C,
   }
 }
 
-static StringRef EnumComplexRangeToStr(LangOptions::ComplexRangeKind Range) {
-  StringRef RangeStr = "";
+static StringRef EnumComplexRangeToStr(LangOptions::ComplexRangeKind Range,
+                                       StringRef Option) {
   switch (Range) {
   case LangOptions::ComplexRangeKind::CX_Limited:
     return "-fcx-limited-range";
@@ -2697,17 +2697,32 @@ static StringRef EnumComplexRangeToStr(LangOptions::ComplexRangeKind Range) {
     return "-fcx-fortran-rules";
     break;
   default:
-    return RangeStr;
+    return Option;
     break;
   }
 }
 
 static void EmitComplexRangeDiag(const Driver &D,
                                  LangOptions::ComplexRangeKind Range1,
-                                 LangOptions::ComplexRangeKind Range2) {
-  if (Range1 != Range2 && Range1 != LangOptions::ComplexRangeKind::CX_None)
-    D.Diag(clang::diag::warn_drv_overriding_option)
-        << EnumComplexRangeToStr(Range1) << EnumComplexRangeToStr(Range2);
+                                 LangOptions::ComplexRangeKind Range2,
+                                 StringRef Option = StringRef()) {
+  if (Range1 != Range2 && Range1 != LangOptions::ComplexRangeKind::CX_None) {
+    bool NegateFortranOption = false;
+    bool NegateLimitedOption = false;
+    if (!Option.empty()) {
+      NegateFortranOption =
+          Range1 == LangOptions::ComplexRangeKind::CX_Fortran &&
+          Option == "fno-cx-fortran-rules";
+      NegateLimitedOption =
+          Range1 == LangOptions::ComplexRangeKind::CX_Limited &&
+          Option == "fno-cx-limited-range";
+    }
+    if (Option.empty() ||
+        !Option.empty() && !NegateFortranOption && !NegateLimitedOption)
+      D.Diag(clang::diag::warn_drv_overriding_option)
+          << EnumComplexRangeToStr(Range1, Option)
+          << EnumComplexRangeToStr(Range2, Option);
+  }
 }
 
 static std::string
@@ -2815,7 +2830,8 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
       break;
     }
     case options::OPT_fno_cx_limited_range:
-      EmitComplexRangeDiag(D, Range, LangOptions::ComplexRangeKind::CX_Full);
+      EmitComplexRangeDiag(D, Range, LangOptions::ComplexRangeKind::CX_Full,
+                           "fno-cx-limited-range");
       Range = LangOptions::ComplexRangeKind::CX_Full;
       break;
     case options::OPT_fcx_fortran_rules: {
@@ -2824,7 +2840,8 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
       break;
     }
     case options::OPT_fno_cx_fortran_rules:
-      EmitComplexRangeDiag(D, Range, LangOptions::ComplexRangeKind::CX_Full);
+      EmitComplexRangeDiag(D, Range, LangOptions::ComplexRangeKind::CX_Full,
+                           "fno-cx-fortran-rules");
       Range = LangOptions::ComplexRangeKind::CX_Full;
       break;
     case options::OPT_ffp_model_EQ: {
diff --git a/clang/test/Driver/range.c b/clang/test/Driver/range.c
index 49116df2f4480e..fd6f5585104e18 100644
--- a/clang/test/Driver/range.c
+++ b/clang/test/Driver/range.c
@@ -12,12 +12,23 @@
 // RUN: %clang -### -target x86_64 -fcx-fortran-rules -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=FRTRN %s
 
+// RUN: %clang -### -target x86_64 -fcx-fortran-rules -c %s 2>&1 \
+// RUN:   -fno-cx-fortran-rules | FileCheck --check-prefix=FULL %s
+
+// RUN: %clang -### -target x86_64 -fcx-fortran-rules -fno-cx-limited-range \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=WARN3 %s
+
 // RUN: %clang -### -target x86_64 -fno-cx-fortran-rules -c %s 2>&1 \
 // RUN:   | FileCheck  %s
 
-// RUN: %clang -### -target x86_64 -fcx-limited-range \
-// RUN: -fcx-fortran-rules -c %s 2>&1 \
-// RUN:   | FileCheck --check-prefix=WARN1 %s
+// RUN: %clang -### -target x86_64 -fcx-limited-range -fcx-fortran-rules \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=WARN1 %s
+
+// RUN: %clang -### -target x86_64 -fcx-limited-range -fno-cx-fortran-rules \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=WARN4 %s
+
+// RUN: %clang -### -target x86_64 -fcx-limited-range -fno-cx-limited-range \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=FULL %s
 
 // RUN: %clang -### -target x86_64 -fcx-fortran-rules \
 // RUN: -fcx-limited-range  -c %s 2>&1 \
@@ -32,8 +43,8 @@
 // RUN: %clang -### -target x86_64 -fcx-limited-range -ffast-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=LMTD %s
 
-// RUN: %clang -### -target x86_64 -ffast-math -fno-cx-limited-range -c %s 2>&1 \
-// RUN:   | FileCheck --check-prefix=FULL %s
+// RUN: %clang -### -target x86_64 -ffast-math -fno-cx-limited-range \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=FULL %s
 
 // RUN: %clang -### -Werror -target x86_64 -fcx-limited-range -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=LMTD %s
@@ -50,3 +61,5 @@
 // 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]
+// WARN3: warning: overriding '-fcx-fortran-rules' option with 'fno-cx-limited-range' [-Woverriding-option]
+// WARN4: warning: overriding '-fcx-limited-range' option with 'fno-cx-fortran-rules' [-Woverriding-option]

@@ -50,3 +61,5 @@
// 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]
// WARN3: warning: overriding '-fcx-fortran-rules' option with 'fno-cx-limited-range' [-Woverriding-option]
// WARN4: warning: overriding '-fcx-limited-range' option with 'fno-cx-fortran-rules' [-Woverriding-option]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have mixed usage of options with and without the option specifier, please add the specifier to the latter option here

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zahiraam zahiraam requested a review from mdtoguchi March 11, 2024 20:13
@zahiraam zahiraam merged commit 0b2c24e into llvm:main Mar 12, 2024
4 checks passed
@zahiraam zahiraam deleted the FixWarningMessage branch March 12, 2024 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants