awarzynski added inline comments.
================
Comment at: flang/lib/Frontend/CompilerInvocation.cpp:88-95
+// Tweak the frontend configuration based on the frontend action
+static void setUpFrontendBasedOnAction(FrontendOptions &opts) {
+ assert(opts.programAction_ != Fortran::frontend::InvalidAction &&
+ "Fortran frontend action not set!");
+
+ if (opts.programAction_ == DebugDumpParsingLog)
+ opts.instrumentedParse_ = true;
----------------
AMDChirag wrote:
> awarzynski wrote:
> > AMDChirag wrote:
> > > Will this function be extended in the future?
> > > If not, an entirely separate function for a couple statements seems
> > > rather overkill.
> > > Will this function be extended in the future?
> >
> > Yes.
> >
> > Ideally we'd want _features_ and _actions_ to be orthogonal and to be
> > controlled by dedicated flags. In this patch, we are adding an _action_
> > flag that toggles a _feature_ option. I think that that's a bit
> > counter-intuitive, but not uncommon or unavoidable long-term.
> >
> > Instead of adding comments, I prefer to introduce a dedicated method for
> > this logic. It will make it easier for us to keep it in one place when new
> > options like this are added. Also, `ParseFrontendArgs` is already quite
> > long and is only going to get longer.
> >
> > Having said all that, we may decide in the future that there's a better way
> > to split this logic. For now I mostly want to avoid extending
> > `ParseFrontendArgs` too much.
> Fair, thank you for explaining!
>
> In such case, shouldn't the function call `setUpFrontendBasedOnAction(opts);`
> be placed after all the fields of the `opts` variable have been set?
Thanks for the suggestion, updated!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97457/new/
https://reviews.llvm.org/D97457
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
