[PATCH] D87989: [Flang][Driver] Add infrastructure for basic frontend actions and file I/O

[Sourabh Singh Tomar via Phabricator via llvm-commits]

SouraVX added a comment.

Thanks for the patch! Overall it's in pretty nice state and conformant to design highlighted. 
Could you please `clang-format` this patch, There are lot of `lint` warning that causes lot of noise while reviewing.
Couple of NIT comments  inline. I've tried marking all but, Could you please finish comments with period.

I think it would good if someone from `clang` community can also take a brief look.



================
Comment at: clang/include/clang/Driver/Options.td:63
 
+// ClangOption - This option should not be accepted by Clang.
+def NoClangOption : OptionFlag;
----------------
 `NoClangOption` ? Is this a Typo, or am I missing the intent behind this ?


================
Comment at: clang/lib/Driver/Driver.cpp:1579
 
-  if (IsFlangMode())
+  if (IsFlangMode()) {
     IncludedFlagsBitmask |= options::FlangOption;
----------------
NIT: May choose to avoid trivial braces. 
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


================
Comment at: flang/include/flang/Frontend/CompilerInstance.h:136
+  /// Add an output file onto the list of tracked output files.
+  ///
+  /// \param outFile - The output file info.
----------------
NIT: Blank line ?


================
Comment at: flang/include/flang/Frontend/CompilerInstance.h:230
+
+  // Allow the frontend compiler to write in the output stream
+  void WriteOutputStream(const std::string &message) {
----------------
NIT: Please finish the comment with a period.


================
Comment at: flang/lib/Frontend/CompilerInstance.cpp:66
+
+  // Create the name of the output file
+  if (!outputPath.empty()) {
----------------
NIT: Missing period at the end ?


================
Comment at: flang/lib/Frontend/CompilerInstance.cpp:67
+  // Create the name of the output file
+  if (!outputPath.empty()) {
+    outFile = std::string(outputPath);
----------------
Can this be simplified ? Maybe a switch case ?


================
Comment at: flang/lib/Frontend/FrontendActions.cpp:25
+
+  // Set/store input file info into compiler instace
+  CompilerInstance &ci = GetCompilerInstance();
----------------
NIT: Instance ? and period at end ?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87989