MaskRay added inline comments.

================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4892
+    } else {
+      D.Diag(diag::warn_drv_unsupported_opt_for_target)
+          << A->getAsString(Args) << TripleStr;
----------------
snehasish wrote:
> MaskRay wrote:
> > This should be an error
> Sure, can you provide a reference/guidance for when err vs warn is 
> appropriate for driver checks? Most of the code in this file uses err but 
> there are some cases of warn being used.
I think a warning is suitable for some GCC specific optimization options where 
a clang port either does not make sense or is not very necessary. This only 
applies to very common options which ease porting to clang. For clang specific 
optimizations, especially  these with complex setup, I  think an error is more 
useful.


================
Comment at: clang/test/Driver/fbasic-block-sections.c:12
 // CHECK-OPT-LABELS: "-fbasic-block-sections=labels"
+// CHECK-TRIPLE-NOT: "-fbasic-block-sections=all"
+// CHECK-TRIPLE:     error: unsupported option '-fbasic-block-sections=all' 
for target
----------------
The NOT pattern is not useful when testing an error.


================
Comment at: clang/test/Driver/fbasic-block-sections.c:5
+// RUN: %clang -### -target x86_64-linux-gnu -fbasic-block-sections=labels %s 
-S 2>&1 | FileCheck -check-prefix=CHECK-OPT-LABELS %s
+// RUN: %clang -### -target arm-unknown-linux -fbasic-block-sections=all %s -S 
2>&1 | FileCheck -check-prefix=CHECK-TRIPLE %s
+// RUN: %clang -### -target x86_64-apple-darwin10 -fbasic-block-sections=all 
%s -S 2>&1 | FileCheck -check-prefix=CHECK-TRIPLE %s
----------------
snehasish wrote:
> MaskRay wrote:
> > `%clang -###` -> `not %clang -fsyntax-only`
> What I really want to check for is the absence of the flag for the particular 
> triple. Using -fsyntax-only means that the cc1 args are not emitted. I've 
> added a check for the diagnostic message but retained `-###` so that I can 
> keep the check for the absence of `-fbasic-block-sections=all`.
Then you can switch to -c

The idea of `not %clang` is that it makes things clear that it is an error.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87426/new/

https://reviews.llvm.org/D87426

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to