sameeranjoshi requested changes to this revision.
sameeranjoshi added a comment.
Thanks for working on it.
A few review comments/questions on changes in `flang` part from the patch.
================
Comment at: flang/include/flang/Frontend/CompilerInstance.h:93
+
+ static clang::IntrusiveRefCntPtr<clang::DiagnosticsEngine> createDiagnostics(
+ clang::DiagnosticOptions *Opts,
----------------
The block of comments above make sense for this function and not the currently
mentioned one.
Please interchange/replace the comments to later declared function.
Wrong comments above could reflect in `doxygen APIs`, misleading the reader of
code.
================
Comment at: flang/include/flang/FrontendTool/Utils.h:1
+
+//===--- Utils.h - Misc utilities for the flang front-end --------*-
C++-*-===//
----------------
`nit:`: blank line.
================
Comment at: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:35
+ if (Flang->getFrontendOpts().ShowVersion) {
+ llvm::cl::PrintVersionMessage();
+ return true;
----------------
With
```
clang --version
clang version 11.0.0 (https://github.com/llvm/llvm-project.git
1acf129bcf9a1b51e301a9fef151254ec4c7ec43)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/local/bin
```
whereas with
```
./bin/flang-new --vesion
Flang experimental driver (flang-new)
```
I see both `clang` & `flang` call
`llvm::cl::PrintVersionMessage()` internally.
Is more information need to be registered in llvm(`llvm::cl`) for flang to give
more detailed output or will that come later once we start adding more patch
for driver?
================
Comment at: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:39
+
+ return true;
+}
----------------
The comment in header for `ExecuteCompilerInvocation` mentions,
```
/// \return - True on success.
bool ExecuteCompilerInvocation(CompilerInstance *Flang);
```
Do we need to have a `false` somewhere here?
I see 2 scenarios when `ExecuteCompilerInvocation` might fail (there could
definitely be more) and there we need an indication of failure by returning
`false`,
1. When there is no actual execution of compiler.
2. The compilation in not successful.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86089/new/
https://reviews.llvm.org/D86089
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
