Skip to content

Check for unsupported target options even with -Qunused-arguments #141698

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

Merged
merged 2 commits into from
May 28, 2025

Conversation

MatzeB
Copy link
Contributor

@MatzeB MatzeB commented May 28, 2025

Fix clang accidentally skipping checks for err_drv_unsupported_opt_for_target when -Qunused-arguments was active.

@MatzeB MatzeB marked this pull request as ready for review May 28, 2025 01:25
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels May 28, 2025
@llvmbot
Copy link
Member

llvmbot commented May 28, 2025

@llvm/pr-subscribers-clang-driver

Author: Matthias Braun (MatzeB)

Changes

Fix clang accidentally skipping checks for err_drv_unsupported_opt_for_target when -Qunused-arguments was active.


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

2 Files Affected:

  • (modified) clang/lib/Driver/Driver.cpp (+4-7)
  • (modified) clang/test/Driver/unsupported-option.c (+7)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index a5a0393ad7912..8d16d3aeeefb6 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -5362,12 +5362,6 @@ void Driver::BuildJobs(Compilation &C) const {
     });
   }
 
-  // If the user passed -Qunused-arguments or there were errors, don't warn
-  // about any unused arguments.
-  if (Diags.hasErrorOccurred() ||
-      C.getArgs().hasArg(options::OPT_Qunused_arguments))
-    return;
-
   // Claim -fdriver-only here.
   (void)C.getArgs().hasArg(options::OPT_fdriver_only);
   // Claim -### here.
@@ -5420,7 +5414,10 @@ void Driver::BuildJobs(Compilation &C) const {
             !C.getActions().empty()) {
           Diag(diag::err_drv_unsupported_opt_for_target)
               << A->getSpelling() << getTargetTriple();
-        } else {
+        } else if (!Diags.hasErrorOccurred() &&
+                   !C.getArgs().hasArg(options::OPT_Qunused_arguments)) {
+          // If the user passed -Qunused-arguments or there were errors, don't
+          // warn about any unused arguments.
           Diag(clang::diag::warn_drv_unused_argument)
               << A->getAsString(C.getArgs());
         }
diff --git a/clang/test/Driver/unsupported-option.c b/clang/test/Driver/unsupported-option.c
index af836cf003374..f996772702abc 100644
--- a/clang/test/Driver/unsupported-option.c
+++ b/clang/test/Driver/unsupported-option.c
@@ -27,3 +27,10 @@
 // RUN: not %clang --target=x86_64 -### -mhtm -lc %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=UNSUP_OPT
 // UNSUP_OPT: error: unsupported option
+
+
+// RUN: not %clang -c --target=aarch64-- -mfpu=crypto-neon-fp-armv8 %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=AARCH64
+// RUN: not %clang -c -Qunused-arguments --target=aarch64-- -mfpu=crypto-neon-fp-armv8 %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=AARCH64
+// AARCH64: error: unsupported option '-mfpu=' for target 'aarch64--'

@llvmbot
Copy link
Member

llvmbot commented May 28, 2025

@llvm/pr-subscribers-clang

Author: Matthias Braun (MatzeB)

Changes

Fix clang accidentally skipping checks for err_drv_unsupported_opt_for_target when -Qunused-arguments was active.


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

2 Files Affected:

  • (modified) clang/lib/Driver/Driver.cpp (+4-7)
  • (modified) clang/test/Driver/unsupported-option.c (+7)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index a5a0393ad7912..8d16d3aeeefb6 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -5362,12 +5362,6 @@ void Driver::BuildJobs(Compilation &C) const {
     });
   }
 
-  // If the user passed -Qunused-arguments or there were errors, don't warn
-  // about any unused arguments.
-  if (Diags.hasErrorOccurred() ||
-      C.getArgs().hasArg(options::OPT_Qunused_arguments))
-    return;
-
   // Claim -fdriver-only here.
   (void)C.getArgs().hasArg(options::OPT_fdriver_only);
   // Claim -### here.
@@ -5420,7 +5414,10 @@ void Driver::BuildJobs(Compilation &C) const {
             !C.getActions().empty()) {
           Diag(diag::err_drv_unsupported_opt_for_target)
               << A->getSpelling() << getTargetTriple();
-        } else {
+        } else if (!Diags.hasErrorOccurred() &&
+                   !C.getArgs().hasArg(options::OPT_Qunused_arguments)) {
+          // If the user passed -Qunused-arguments or there were errors, don't
+          // warn about any unused arguments.
           Diag(clang::diag::warn_drv_unused_argument)
               << A->getAsString(C.getArgs());
         }
diff --git a/clang/test/Driver/unsupported-option.c b/clang/test/Driver/unsupported-option.c
index af836cf003374..f996772702abc 100644
--- a/clang/test/Driver/unsupported-option.c
+++ b/clang/test/Driver/unsupported-option.c
@@ -27,3 +27,10 @@
 // RUN: not %clang --target=x86_64 -### -mhtm -lc %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=UNSUP_OPT
 // UNSUP_OPT: error: unsupported option
+
+
+// RUN: not %clang -c --target=aarch64-- -mfpu=crypto-neon-fp-armv8 %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=AARCH64
+// RUN: not %clang -c -Qunused-arguments --target=aarch64-- -mfpu=crypto-neon-fp-armv8 %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=AARCH64
+// AARCH64: error: unsupported option '-mfpu=' for target 'aarch64--'

- Address review feedback
- Change logic to check for `HasError()` before the loop again to be
  closer to the original behavior; fixes `x86-target-features.c`.
@MatzeB MatzeB merged commit a4b2f4a into llvm:main May 28, 2025
8 of 9 checks passed
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
…vm#141698)

Fix clang accidentally skipping checks for
`err_drv_unsupported_opt_for_target` when `-Qunused-arguments` was
active.
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.

3 participants