awarzynski added a comment.

Thank you for submitting this @arnamoy10 ! Comments inline.


================
Comment at: clang/include/clang/Driver/Options.td:4232-4233
   HelpText<"Use INTEGER(KIND=8) for the result type in size-related 
intrinsics.">;
+def fdebug_module_writer : Flag<["-"],"fdebug-module-writer">, Group<f_Group>, 
+  HelpText<"Enables showing debug messages while writing module files.">;
 }
----------------
IMO this should be a frontend-only option. Compiler end-users are unlikely to 
need this in their regular workflows, right?  I recommend moving it [[ 
https://github.com/llvm/llvm-project/blob/adfd3c7083f9808d145239153c10f72eece485d8/clang/include/clang/Driver/Options.td#L4262-L4272
 | here ]].


================
Comment at: flang/test/Semantics/mod-file-rewriter.f90:9
+! RUN: %flang-new -fsyntax-only -fdebug-module-writer 
%p/Inputs/mod-file-unchanged.f90 2>&1 | FileCheck %s --check-prefix 
CHECK_UNCHANGED
+! RUN: %flang-new -fsyntax-only -fdebug-module-writer 
%p/Inputs/mod-file-changed.f90 2>&1 | FileCheck %s --check-prefix CHECK_CHANGED
 
----------------
arnamoy10 wrote:
> tskeith wrote:
> > Why do you need to duplicate these lines? Doesn't it work to use `%flang`?
> `%flang` will unfortunately invoke `f18` currently.
That depends on the value of `FLANG_BUILD_NEW_DRIVER`, which is translated into 
`include_flang_new_driver_test` [[ 
https://github.com/llvm/llvm-project/blob/adfd3c7083f9808d145239153c10f72eece485d8/flang/test/lit.cfg.py#L74-L85
 | here ]]. I also think that this should work fine with `%flang`.

Also, I suggest that we only have tests with `flang_fc1` and keep this as a 
frontend-only option.


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

https://reviews.llvm.org/D96875

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

Reply via email to