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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits