-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[flang][driver] Do not allow -module-dir to be joined to the value #168748
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
In f820625, the -module-dir option was modified to allow it to be joined to the argument value, i.e. `-module-dir<value>` was allowed in addition to `-module-dir <value>`. While this is consistent with its aliasee, '-J', it is not consistent with the way other long options are handled in flang. This also makes it difficult to add another option such as -module-directives since it is not clear if "ectives" is the name of a directory containing .mod files. `-module-dir<value>` is no longer allowed. Only `-module-dir <value>` may be used. `-module-dir=<value>` is not allowed either. However, both `-J<value>` and `-J <value>` are permitted. `-J` was made into the "primary" option and `-module-dir` was made its aliasee instead of the other way around since the former can be both joined and separated while the latter can only be separate. Fixes llvm#168740
|
@llvm/pr-subscribers-clang-driver Author: Tarun Prabhu (tarunprabhu) ChangesIn f820625, the -module-dir option was modified to allow it to be joined to the argument value, i.e.
Fixes #168740 Full diff: https://github.com/llvm/llvm-project/pull/168748.diff 4 Files Affected:
diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td
index 786acd6abbd21..40a6d0d8c2137 100644
--- a/clang/include/clang/Options/Options.td
+++ b/clang/include/clang/Options/Options.td
@@ -7236,11 +7236,14 @@ def cpp : Flag<["-"], "cpp">, Group<f_Group>,
HelpText<"Enable predefined and command line preprocessor macros">;
def nocpp : Flag<["-"], "nocpp">, Group<f_Group>,
HelpText<"Disable predefined and command line preprocessor macros">;
-def module_dir : JoinedOrSeparate<["-"], "module-dir">, MetaVarName<"<dir>">,
+def J : JoinedOrSeparate<["-"], "J">,
+ Flags<[RenderJoined]>,
HelpText<"Put MODULE files in <dir>">,
DocBrief<[{This option specifies where to put .mod files for compiled modules.
It is also added to the list of directories to be searched by an USE statement.
The default is the current directory.}]>;
+def module_dir : Separate<["-"], "module-dir">, MetaVarName<"<dir>">,
+ Alias<J>;
def ffixed_form : Flag<["-"], "ffixed-form">, Group<f_Group>,
HelpText<"Process source files in fixed form">;
@@ -7376,11 +7379,6 @@ def fdo_concurrent_to_openmp_EQ : Joined<["-"], "fdo-concurrent-to-openmp=">,
Values<"none, host, device">;
} // let Visibility = [FC1Option, FlangOption]
-def J : JoinedOrSeparate<["-"], "J">,
- Flags<[RenderJoined]>, Visibility<[FlangOption, FC1Option]>,
- Group<gfortran_Group>,
- Alias<module_dir>;
-
//===----------------------------------------------------------------------===//
// FC1 Options
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 270904de544d6..b49cf0f3ce2bb 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -126,7 +126,7 @@ void Flang::addDebugOptions(const llvm::opt::ArgList &Args, const JobAction &JA,
const auto &TC = getToolChain();
const Driver &D = TC.getDriver();
Args.addAllArgs(CmdArgs,
- {options::OPT_module_dir, options::OPT_fdebug_module_writer,
+ {options::OPT_J, options::OPT_fdebug_module_writer,
options::OPT_fintrinsic_modules_path, options::OPT_pedantic,
options::OPT_std_EQ, options::OPT_W_Joined,
options::OPT_fconvert_EQ, options::OPT_fpass_plugin_EQ,
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 893121fe01f27..94cf50747ac65 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -959,7 +959,7 @@ static bool parseSemaArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
// -J/module-dir option
std::vector<std::string> moduleDirList =
- args.getAllArgValues(clang::options::OPT_module_dir);
+ args.getAllArgValues(clang::options::OPT_J);
// User can only specify one -J/-module-dir directory, but may repeat
// -J/-module-dir as long as the directory is the same each time.
// https://gcc.gnu.org/onlinedocs/gfortran/Directory-Options.html
diff --git a/flang/test/Driver/write-module.f90 b/flang/test/Driver/write-module.f90
index c4dbaddd4b270..ddced1342f025 100644
--- a/flang/test/Driver/write-module.f90
+++ b/flang/test/Driver/write-module.f90
@@ -3,6 +3,19 @@
! * is saved in the _directory specified by the user_
! We use `-fsyntax-only` as it stops after the semantic checks (the module file is generated when sema checks are run)
+! ------------------------------------------------------------------------------
+! At one time, flang accepted -module-dir<value> (note the lack of a separator
+! between -module-dir and <value>). This is no longer allowed.
+! -module-dir=<value> is also not allowed
+!
+! RUN: not %flang -fsyntax-only -module-dir%t %s 2>&1 \
+! RUN: | FileCheck %s -check-prefix=JOINED
+!
+! RUN: not %flang -fsyntax-only -module-dir=%t %s 2>&1 \
+! RUN: | FileCheck %s -check-prefix=JOINED
+!
+! JOINED: error: unknown argument: '-module-dir{{.+}}'
+!
!--------------------------
! -module-dir <value>
!--------------------------
@@ -10,15 +23,7 @@
! RUN: cd %t && %flang -fsyntax-only -module-dir %t/dir-flang %s
! RUN: ls %t/dir-flang/testmodule.mod && not ls %t/testmodule.mod
! RUN: cd -
-
-!--------------------------
-! -module-dir<value>
-!--------------------------
-! RUN: rm -rf %t && mkdir -p %t/dir-flang
-! RUN: cd %t && %flang -fsyntax-only -module-dir%t/dir-flang %s
-! RUN: ls %t/dir-flang/testmodule.mod && not ls %t/testmodule.mod
-! RUN: cd -
-
+!
!---------------------------
! -J <value>
!---------------------------
|
|
@llvm/pr-subscribers-flang-driver Author: Tarun Prabhu (tarunprabhu) ChangesIn f820625, the -module-dir option was modified to allow it to be joined to the argument value, i.e.
Fixes #168740 Full diff: https://github.com/llvm/llvm-project/pull/168748.diff 4 Files Affected:
diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td
index 786acd6abbd21..40a6d0d8c2137 100644
--- a/clang/include/clang/Options/Options.td
+++ b/clang/include/clang/Options/Options.td
@@ -7236,11 +7236,14 @@ def cpp : Flag<["-"], "cpp">, Group<f_Group>,
HelpText<"Enable predefined and command line preprocessor macros">;
def nocpp : Flag<["-"], "nocpp">, Group<f_Group>,
HelpText<"Disable predefined and command line preprocessor macros">;
-def module_dir : JoinedOrSeparate<["-"], "module-dir">, MetaVarName<"<dir>">,
+def J : JoinedOrSeparate<["-"], "J">,
+ Flags<[RenderJoined]>,
HelpText<"Put MODULE files in <dir>">,
DocBrief<[{This option specifies where to put .mod files for compiled modules.
It is also added to the list of directories to be searched by an USE statement.
The default is the current directory.}]>;
+def module_dir : Separate<["-"], "module-dir">, MetaVarName<"<dir>">,
+ Alias<J>;
def ffixed_form : Flag<["-"], "ffixed-form">, Group<f_Group>,
HelpText<"Process source files in fixed form">;
@@ -7376,11 +7379,6 @@ def fdo_concurrent_to_openmp_EQ : Joined<["-"], "fdo-concurrent-to-openmp=">,
Values<"none, host, device">;
} // let Visibility = [FC1Option, FlangOption]
-def J : JoinedOrSeparate<["-"], "J">,
- Flags<[RenderJoined]>, Visibility<[FlangOption, FC1Option]>,
- Group<gfortran_Group>,
- Alias<module_dir>;
-
//===----------------------------------------------------------------------===//
// FC1 Options
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 270904de544d6..b49cf0f3ce2bb 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -126,7 +126,7 @@ void Flang::addDebugOptions(const llvm::opt::ArgList &Args, const JobAction &JA,
const auto &TC = getToolChain();
const Driver &D = TC.getDriver();
Args.addAllArgs(CmdArgs,
- {options::OPT_module_dir, options::OPT_fdebug_module_writer,
+ {options::OPT_J, options::OPT_fdebug_module_writer,
options::OPT_fintrinsic_modules_path, options::OPT_pedantic,
options::OPT_std_EQ, options::OPT_W_Joined,
options::OPT_fconvert_EQ, options::OPT_fpass_plugin_EQ,
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 893121fe01f27..94cf50747ac65 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -959,7 +959,7 @@ static bool parseSemaArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
// -J/module-dir option
std::vector<std::string> moduleDirList =
- args.getAllArgValues(clang::options::OPT_module_dir);
+ args.getAllArgValues(clang::options::OPT_J);
// User can only specify one -J/-module-dir directory, but may repeat
// -J/-module-dir as long as the directory is the same each time.
// https://gcc.gnu.org/onlinedocs/gfortran/Directory-Options.html
diff --git a/flang/test/Driver/write-module.f90 b/flang/test/Driver/write-module.f90
index c4dbaddd4b270..ddced1342f025 100644
--- a/flang/test/Driver/write-module.f90
+++ b/flang/test/Driver/write-module.f90
@@ -3,6 +3,19 @@
! * is saved in the _directory specified by the user_
! We use `-fsyntax-only` as it stops after the semantic checks (the module file is generated when sema checks are run)
+! ------------------------------------------------------------------------------
+! At one time, flang accepted -module-dir<value> (note the lack of a separator
+! between -module-dir and <value>). This is no longer allowed.
+! -module-dir=<value> is also not allowed
+!
+! RUN: not %flang -fsyntax-only -module-dir%t %s 2>&1 \
+! RUN: | FileCheck %s -check-prefix=JOINED
+!
+! RUN: not %flang -fsyntax-only -module-dir=%t %s 2>&1 \
+! RUN: | FileCheck %s -check-prefix=JOINED
+!
+! JOINED: error: unknown argument: '-module-dir{{.+}}'
+!
!--------------------------
! -module-dir <value>
!--------------------------
@@ -10,15 +23,7 @@
! RUN: cd %t && %flang -fsyntax-only -module-dir %t/dir-flang %s
! RUN: ls %t/dir-flang/testmodule.mod && not ls %t/testmodule.mod
! RUN: cd -
-
-!--------------------------
-! -module-dir<value>
-!--------------------------
-! RUN: rm -rf %t && mkdir -p %t/dir-flang
-! RUN: cd %t && %flang -fsyntax-only -module-dir%t/dir-flang %s
-! RUN: ls %t/dir-flang/testmodule.mod && not ls %t/testmodule.mod
-! RUN: cd -
-
+!
!---------------------------
! -J <value>
!---------------------------
|
🐧 Linux x64 Test Results
All tests passed but another part of the build failed. Click on a failure below to see the details. flang-rt/lib/runtime/CMakeFiles/flang_rt.runtime.static.dir/home/gha/actions-runner/_work/llvm-project/llvm-project/flang/module/iso_fortran_env_impl.f90-pp.f90 flang-rt/lib/runtime/CMakeFiles/flang_rt.runtime.static.dir/home/gha/actions-runner/_work/llvm-project/llvm-project/flang/module/iso_fortran_env_impl.f90.o.ddiIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
|
As much as I would like this, I fear CMake has to be fixed first: https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/Compiler/LLVMFlang-Fortran.cmake?ref_type=heads#L10 |
I am happy to do that. It's probably better to weed this out if we can because it may well cause us obscure problems in the future. Replacing the -module-dir in Do you have any suggestions on how to go about it? If we do push the change to |
In f820625, the -module-dir option was modified to allow it to be joined to the argument value, i.e.
-module-dir<value>was allowed in addition to-module-dir <value>. While this is consistent with its aliasee, '-J', it is not consistent with the way other long options are handled in flang. This also makes it difficult to add another option such as -module-directives since it is not clear if "ectives" is the name of a directory containing .mod files.-module-dir<value>is no longer allowed. Only-module-dir <value>may be used.-module-dir=<value>is not allowed either. However, both-J<value>and-J <value>are permitted.-Jwas made into the "primary" option and-module-dirwas made its aliasee instead of the other way around since the former can be both joined and separated while the latter can only be separate.Fixes #168740