[PATCH] D98657: [flang][driver] Add options for -Werror

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

Thank you for adding this  @arnamoy10 !

I think that in order to match the semantics of `-Werror` from `f18`, this patch needs to be extended a bit. In the following two cases, `f18` will exit immediately, whereas `flang-new -fc1` will happily carry-on:

- https://github.com/llvm/llvm-project/blob/main/flang/tools/f18/f18.cpp#L212-L213
- https://github.com/llvm/llvm-project/blob/main/flang/tools/f18/f18.cpp#L241-L242

Could you try implementing similar semantics here?

I've left a few more comments inline.



================
Comment at: flang/lib/Frontend/CompilerInvocation.cpp:331-334
+  // -Werror option
+  if (args.hasArg(clang::driver::options::OPT_W_Joined)) {
+    res.SetWarnAsErr(true);
+  }
----------------
1. This will capture almost anything that matches `-W<warning-flag>`. Could you verify that it's indeed `-Werror`?
2. This is a diagnostics rather than `Sema` option. We are likely to introduce much more with time. Could you move it to a separate method?


================
Comment at: flang/test/Semantics/dosemantics03.f90:1-2
 ! RUN: %S/test_errors.sh %s %t %f18 -Mstandard -Werror
+! RUN: %S/test_errors.sh %s %t %flang_fc1 -Werror
 
----------------
Rather than adding the 2nd line, could you update the first line to use `%flang_fc1` instead of `%f18`?


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

https://reviews.llvm.org/D98657