[PATCH] D142757: [clang][driver] Emit an error for `/clang:-x`

Mariya Podchishchaeva via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 30 02:51:59 PST 2023


Fznamznon marked an inline comment as done.
Fznamznon added inline comments.


================
Comment at: clang/lib/Driver/Driver.cpp:2571
     // No driver mode exposes -x and /TC or /TP; we don't support mixing them.
     assert(!Args.hasArg(options::OPT_x) && "-x and /TC or /TP is not allowed");
   }
----------------
MaskRay wrote:
> ```
> -    assert(!Args.hasArg(options::OPT_x) && "-x and /TC or /TP is not allowed");
> +    if (auto *A = Args.getLastArg(options::OPT_x))
> +      Diag(diag::err_drv_unsupported_opt_with_suggestion)
> +          << A->getAsString(Args) << "/TC' or '/TP";
> ```
Thank you, the suggestion seems reasonable to me. Although, the concrete place doesn't.
The suggested line to change is under if (we have /TC or /TP argument), so if I put error emission there it won't be emitted if no /TC or /TP was passed and the original problem reported in https://github.com/llvm/llvm-project/issues/59307 won't be resolved. People will still be confused by the warning saying `-x c` passed after last input when in fact they passed `/clang:-x` before the input.

Also, the whole function doesn't return even if diagnostic is emitted, so I put error emission in a way to not emit a warning saying that -x is passed after last input together with it, because both errors emitted like this:

error: '-x c' after last input file has no effect
error: unsupported option '-x c'; did you mean '/TC' or '/TP'?

look confusing.


================
Comment at: clang/test/Driver/x-args.c:8
 // CHECK: '-x c++' after last input file has no effect
+//
+// RUN: %clang_cl /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | FileCheck -check-prefix=CL %s
----------------
MaskRay wrote:
> Delete `//`
> 
Done, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142757



More information about the cfe-commits mailing list