[PATCH] D96407: [flang][driver] Add extension options and -finput-charset

Andrzej Warzynski via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 12 08:25:16 PST 2021


awarzynski added a comment.

Thank you for submitting this @FarisRehman !

Overall this looks good to me. I've left a few minor comments inline. Also, I think that it is worth adding a help message for `-finput-charset`. Lack of it in `clang` feels like an accidental omission. We can follow GCC here:

  $ gcc --help=joined | grep input-charset
    -finput-charset=<cset>      Specify the default character set for source files.



================
Comment at: flang/test/Flang-Driver/implicit-none.f90:9
+! RUN: %flang-new -fsyntax-only %s  2>&1 | FileCheck %s --allow-empty --check-prefix=DEFAULT
+! RUN: not %flang-new -fsyntax-only -fimplicit-none %s  2>&1 | FileCheck %s --check-prefix=ALWAYS
+! RUN: %flang-new -fsyntax-only -fno-implicit-none %s  2>&1 | FileCheck %s --allow-empty --check-prefix=DEFAULT
----------------
[nit] `ALWAYS` is a bit unclear to me. Perhaps `WITH_IMPL_NONE`?


================
Comment at: flang/test/Flang-Driver/implicit-none.f90:10
+! RUN: not %flang-new -fsyntax-only -fimplicit-none %s  2>&1 | FileCheck %s --check-prefix=ALWAYS
+! RUN: %flang-new -fsyntax-only -fno-implicit-none %s  2>&1 | FileCheck %s --allow-empty --check-prefix=DEFAULT
+
----------------
What about:
```
! RUN: %flang-new -fsyntax-only %s  2>&1 | FileCheck %s --allow-empty --check-prefix=DEFAULT
```
and:
```
! RUN: %flang-new -fimplicit-none -fno-implicit-none -fsyntax-only %s  2>&1 | FileCheck %s --allow-empty --check-prefix=DEFAULT
```

Similar point for other tests.


================
Comment at: flang/test/Flang-Driver/implicit-none.f90:28-29
+! ALWAYS:No explicit type declared for 'a'
+! ALWAYS-NEXT:function a()
+! ALWAYS-NEXT:^
+! ALWAYS-NEXT:No explicit type declared for 'b'
----------------
[nit] This particular output might change in the future, which could make this test a bit fragile. I also think that it's not key here (IIUC, lines 27 and 30 are). Perhaps we could skip these?


================
Comment at: flang/test/Flang-Driver/input-charset.f90:1-14
+! Ensure argument -finput-charset is forwarded to the frontend.
+
+! REQUIRES: new-flang-driver
+
+!--------------------------
+! FLANG DRIVER (flang-new)
+!--------------------------
----------------
Could this be merged with `pipeline.f90` from https://reviews.llvm.org/D96344? (btw, IMHO it is worth renaming that file) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96407



More information about the cfe-commits mailing list