[PATCH] D86089: [flang][driver]Add experimental flang driver and frontend with help screen

[sameeran joshi via Phabricator via cfe-commits]

sameeranjoshi added a comment.

Thanks for the work.

A couple of comments on `clang/` related changes:
An `out-of-tree` build with this patch fails for me:
Here's what I did:
I initially used `ENABLE_PROJECTS="clang;mlir"` to build `llvm-project`, I didn't build `flang` during this run.

Then I passed following to the cmake when building `flang` as out of tree.

  cmake -G Ninja \
    -DCMAKE_BUILD_TYPE=Release \
    -DFLANG_ENABLE_WERROR=On \
    -DCMAKE_CXX_STANDARD=17 \
    -DLLVM_TARGETS_TO_BUILD=host \
    -DLLVM_LIT_ARGS=-v \
    -D LLVM_DIR=${LLVM_MLIR_CLANG_BUILD}/lib/cmake/llvm \
    -D MLIR_DIR=${LLVM_MLIR_CLANG_BUILD}/lib/cmake/mlir \
    -DBUILD_FLANG_NEW_DRIVER=ON \
    ../
  ninja -j16

where `LLVM_MLIR_CLANG_BUILD` points to initially built `llvm-project` .
I see below error message:

  ../lib/Frontend/CompilerInvocation.cpp: In static member function ‘static bool flang::CompilerInvocation::CreateFromArgs(flang::CompilerInvocation&, llvm::ArrayRef<const char*>, clang::DiagnosticsEngine&)’:
  ../lib/Frontend/CompilerInvocation.cpp:85:67: error: ‘FlangOption’ is not a member of ‘clang::driver::options’
         clang::driver::options::CC1Option | clang::driver::options::FlangOption;
                                                                     ^~~~~~~~~~~
  ../lib/Frontend/CompilerInvocation.cpp:85:67: note: suggested alternative: ‘LastOption’
         clang::driver::options::CC1Option | clang::driver::options::FlangOption;
                                                                     ^~~~~~~~~~~
                                                                     LastOption

Do you need to add `-DCLANG_DIR` flag, as there seem to be a dependency for this patch on clang as libraries?



================
Comment at: clang/include/clang/Driver/Options.td:60
+// FlangOption - This is considered a "core" Flang option, available in
+// flang mode
+def FlangOption : OptionFlag;
----------------
`nit:` a missing period.
// flang mode -> // flang mode.


================
Comment at: clang/include/clang/Driver/Options.td:2076
 def headerpad__max__install__names : Joined<["-"], "headerpad_max_install_names">;
-def help : Flag<["-", "--"], "help">, Flags<[CC1Option,CC1AsOption]>,
+def help : Flag<["-", "--"], "help">, Flags<[CC1Option,CC1AsOption,FlangOption]>,
   HelpText<"Display available options">;
----------------
`nit:` - A space before `FlangOption`
CC1AsOption,FlangOption -> CC1AsOption, FlangOption


================
Comment at: clang/include/clang/Driver/Options.td:3019
 // We give --version different semantics from -version.
-def _version : Flag<["--"], "version">, Flags<[CoreOption, CC1Option]>,
+def _version : Flag<["--"], "version">, Flags<[CoreOption, CC1Option,FlangOption]>,
   HelpText<"Print version information">;
----------------
Same as above.
nit: - A space before `FlangOption`


================
Comment at: clang/lib/Driver/Driver.cpp:1569
 
+  if (Mode == DriverMode::FlangMode) {
+    ExcludedFlagsBitmask |= options::CLOption;
----------------
Can `IsFlangMode()` be used instead?


================
Comment at: clang/lib/Driver/Driver.cpp:1573
+    ExcludedFlagsBitmask |= options::CC1Option;
+    IncludedFlagsBitmask |= options::FlangOption;
+  } else
----------------
In `enum ClangFlags` 
inside File `clang/include/clang/Driver/Options.h`
there are various other options do they need to be considered?
If not, how are they handled?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86089