tskeith requested changes to this revision.
tskeith added inline comments.
This revision now requires changes to proceed.
================
Comment at: flang/include/flang/Frontend/CompilerInstance.h:11
+
+#include "flang/Frontend/CompilerInvocation.h"
+
----------------
Why is this called "Frontend" rather than "Driver"?
================
Comment at: flang/include/flang/Frontend/CompilerInstance.h:16
+
+namespace flang {
+
----------------
Nothing else is in namespace `flang`. `Fortran::frontend` would be more
consistent.
================
Comment at: flang/include/flang/Frontend/CompilerInstance.h:21
+ /// The options used in this compiler instance.
+ std::shared_ptr<CompilerInvocation> Invocation;
+
----------------
Data members, local variables, etc. should start with lower case.
Non-public data members should end with underscore.
Please follow the style in flang/documentation/C++style.md.
================
Comment at: flang/include/flang/Frontend/FrontendOptions.h:31
+ Language Lang;
+ unsigned Fmt : 3;
+ unsigned Preprocessed : 1;
----------------
Why isn't the type `Format`?
================
Comment at: flang/include/flang/Frontend/FrontendOptions.h:32
+ unsigned Fmt : 3;
+ unsigned Preprocessed : 1;
+
----------------
Why isn't the type `bool`?
================
Comment at: flang/include/flang/Frontend/FrontendOptions.h:65
+ /// Show the -version text.
+ unsigned ShowVersion : 1;
+
----------------
Why aren't the types `bool`?
================
Comment at: flang/include/flang/FrontendTool/Utils.h:18
+
+#include <memory>
+
----------------
Is this needed?
================
Comment at: flang/lib/Frontend/CompilerInstance.cpp:39
+ } else
+ Diags->setClient(new clang::TextDiagnosticPrinter(llvm::errs(), Opts));
+
----------------
The "then" and "else" statements should have braces around them.
================
Comment at: flang/lib/Frontend/CompilerInvocation.cpp:46
+ InputKind DashX(Language::Unknown);
+ return DashX;
+ }
----------------
Why not `return InputKind{Language::Unknown};`?
================
Comment at: flang/lib/Frontend/CompilerInvocation.cpp:91
+ InputKind DashX = ParseFrontendArgs(Res.getFrontendOpts(), Args, Diags);
+ (void)DashX;
+
----------------
What is the purpose of `DashX` here?
================
Comment at: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:20
+
+using namespace flang;
+namespace flang {
----------------
What is this for?
================
Comment at: flang/tools/flang-driver/driver.cpp:30
+extern int fc1_main(
+ llvm::ArrayRef<const char *> Argv, const char *Argv0, void *MainAddr);
+
----------------
Why isn't this declared in a header?
================
Comment at: flang/tools/flang-driver/fc1_main.cpp:32
+int fc1_main(
+ llvm::ArrayRef<const char *> Argv, const char *Argv0, void *MainAddr) {
+
----------------
MainAddr isn't used.
================
Comment at: flang/tools/flang-driver/fc1_main.cpp:54
+ // Execute the frontend actions.
+ { Success = ExecuteCompilerInvocation(Flang.get()); }
+
----------------
Why is this statement in a block? Is the result of CreateFromArgs supposed to
affect the return value of this function?
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
