[PATCH] D95448: [flang][driver] Add support for `-J/-module-dir`

[Tim Keith via Phabricator via cfe-commits]

tskeith added inline comments.


================
Comment at: clang/include/clang/Driver/Options.td:1018
+  an USE statement.  The default is the current directory.}]>,Group<gfortran_Group>;
+def module_dir : Separate<["-"], "module-dir">, Flags<[FlangOption,FC1Option]>, MetaVarName<"<dir>">, Alias<J>;
 def dsym_dir : JoinedOrSeparate<["-"], "dsym-dir">,
----------------
It would be better to make `-module-dir` the main option and `-J` the alias. `-J` is only there for gfortran compatibility, not because it is a good name for the option.


================
Comment at: flang/include/flang/Frontend/CompilerInstance.h:105
   /// {
+  Fortran::semantics::SemanticsContext &semaChecking() const { return *semantics_; }
 
----------------
`semanticsContext` would be a better name for this function.


================
Comment at: flang/include/flang/Frontend/CompilerInvocation.h:67
+  // of options.
+  std::string moduleDirJ_ = "."; 
+
----------------
`moduleDir_` would be a better name.


================
Comment at: flang/lib/Frontend/CompilerInstance.cpp:29
+      semantics_(new Fortran::semantics::SemanticsContext(*(new  Fortran::common::IntrinsicTypeDefaultKinds()),*(new common::LanguageFeatureControl()),
+                 *allCookedSources_)) {
 
----------------
Why is `semantics_` a `shared_ptr` rather than a simple data member of  type `SemanticsContext`? It's owned by only `CompilerInstance`, not shared.
The same question probably applies to the other fields too.


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

https://reviews.llvm.org/D95448