[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