awarzynski accepted this revision.
awarzynski added a comment.
This revision is now accepted and ready to land.

Thank you for updating this @arnamoy10!

One thing worth pointing out - this patch adds `-pedantic` rather than 
`-fpendatic` as @richard.barton.arm suggested. That was clearly a typo, so 
everything is good.

IMO this is ready to land provided that:

- the test is updated to work with `f18`
- summary/commit message is updated to reflect the recent changes (i.e. that 
this patch adds `-std=f2018` AND `-pedantic`)

These are small changes and I'm happy for you to apply them when merging 
(rather than updating here first).



================
Comment at: flang/lib/Frontend/CompilerInvocation.cpp:375
+
+  //-fpedantic
+  if (args.hasArg(clang::driver::options::OPT_pedantic)) {
----------------
[nit] `-pedantic` instead


================
Comment at: flang/lib/Frontend/CompilerInvocation.cpp:535
+
+  // Set the standard
+  if (enableConformanceChecks()) {
----------------
[nit] Comment inconsistent with the code (no standard is set here)


================
Comment at: flang/test/Driver/std2018.f90:6-8
+! RUN: %flang_fc1 %s  2>&1 | FileCheck %s --allow-empty --check-prefix=WITHOUT
+! RUN: %flang_fc1 -std=f2018 %s  2>&1 | FileCheck %s --check-prefix=GIVEN
+! RUN: %flang_fc1 -pedantic %s  2>&1 | FileCheck %s --check-prefix=GIVEN
----------------
`flang-new -fc1` (akin to `clang- cc1`), runs `-fsyntax-only` when no other 
action flag is specified. `f18` will go straight into code-generation. As such, 
this test fails for `f18`. Adding `-fsyntax-only` should fix it. It will also 
make the test a bit more clearer.


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

https://reviews.llvm.org/D97119

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

Reply via email to