arnamoy10 added a comment. Sounds good to me, thanks for the help. In the meantime I will work on the `fdefault-*` family of flags, which will be dependent on this patch I think.
================ Comment at: clang/include/clang/Driver/Options.td:4124 def A_DASH : Joined<["-"], "A-">, Group<gfortran_Group>; -def J : JoinedOrSeparate<["-"], "J">, Flags<[RenderJoined]>, Group<gfortran_Group>; def cpp : Flag<["-"], "cpp">, Group<gfortran_Group>; ---------------- awarzynski wrote: > There's no need to move this, is there? I feel that it's better to keep all > `gfortran` options together. This needs to be moved, as we are using Aliases. The way Aliases work is (in this order) (1) you create the base option (that ================ 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">, ---------------- tskeith wrote: > 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. Sure, we can do that. I just chose -J to be default as it is easier to type for the user :) ================ Comment at: flang/lib/Frontend/CompilerInstance.cpp:29 + semantics_(new Fortran::semantics::SemanticsContext(*(new Fortran::common::IntrinsicTypeDefaultKinds()),*(new common::LanguageFeatureControl()), + *allCookedSources_)) { ---------------- tskeith wrote: > 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. No idea about this design decision. I have not found any other DS that is "sharing" these pointers. I can take them out. ================ Comment at: flang/lib/Frontend/CompilerInvocation.cpp:302 + +void CompilerInvocation::setSemanticsOpts(Fortran::semantics::SemanticsContext &semaCtxt) { + auto &fortranOptions = fortranOpts(); ---------------- awarzynski wrote: > The argument is not needed here, is it? You could just write: > ``` > auto &semaCtx = semanticsContext() > ``` > Or am I missing something? `semanticsContext_` belongs to `CompilerInstance`, not `CompilerInvocation`, so unfortunately that is not possible. ================ Comment at: flang/test/Flang-Driver/include-module.f90:11 ! RUN: not %flang-new -fsyntax-only -I %S/Inputs %s 2>&1 | FileCheck %s --check-prefix=SINGLEINCLUDE +! RUN: not %flang-new -fsyntax-only -J %S/Inputs -I %S/Inputs/module-dir %s 2>&1 | FileCheck %s --check-prefix=INCLUDED +! RUN: not %flang-new -fsyntax-only -J %S/Inputs %s 2>&1 | FileCheck %s --check-prefix=SINGLEINCLUDE ---------------- awarzynski wrote: > What about: > * `-J %S/Inputs -J %S/Inputs/module-dir` (i.e. `-J` + `-J`) > * `-J %S/Inputs -module-dir %S/Inputs/module-dir` (i.e. `-J` + `-module-dir`) > * `-module-dir %S/Inputs -J%S/Inputs/module-dir` (i.e. `-module-dir` + `-J`) > * `-module-dir %S/Inputs -I%S/Inputs/module-dir` (.e. `-module-dir` + `-I`) > > I appreciate that this is a bit tedious, but IMO it is worthwhile to add a > few more cases here to avoid any unpleasant breakage in the future. Also - > what should the relation between `-I` and `-J` be here? As in, which ought to > have higher priority? Done CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95448/new/ https://reviews.llvm.org/D95448 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits