[flang][driver] deprecate manual usage of -lFortran_main #79016

tblah · 2024-01-22T17:33:19Z

Intended to warn users of the 18.x release not to do this.

A better solution should be found for the 19.x release. See discussion in #78152.

Unfortunately there is no warning on Windows currently. I am rushing to get this landed before 19.x branches.

Intended to warn users of the 19.x release not to do this. A better solution should be found for the 20.x release. See discussion in llvm#78152. Unfortunately there is no warning on Windows currently. I am rushing to get this landed before 19.x branches.

llvmbot · 2024-01-22T17:33:49Z

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-flang-driver

@llvm/pr-subscribers-clang-driver

Author: Tom Eccles (tblah)

Changes

Intended to warn users of the 19.x release not to do this.

A better solution should be found for the 20.x release. See discussion in #78152.

Unfortunately there is no warning on Windows currently. I am rushing to get this landed before 19.x branches.

Full diff: https://github.com/llvm/llvm-project/pull/79016.diff

3 Files Affected:

(modified) clang/include/clang/Basic/DiagnosticDriverKinds.td (+2)
(modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+12-2)
(modified) flang/test/Driver/linker-flags.f90 (+4)

diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 090b169a0e72408..094fe1950941270 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -427,6 +427,8 @@ def warn_drv_clang_unsupported : Warning<
   "the clang compiler does not support '%0'">;
 def warn_drv_deprecated_arg : Warning<
   "argument '%0' is deprecated, use '%1' instead">, InGroup<Deprecated>;
+def warn_drv_deprecated_custom : Warning<
+  "argument '%0' is deprecated, %1">, InGroup<Deprecated>;
 def warn_drv_assuming_mfloat_abi_is : Warning<
   "unknown platform, assuming -mfloat-abi=%0">;
 def warn_drv_unsupported_float_abi_by_lib : Warning<
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 385f66f3782bc1a..938f4941eb5a03f 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1193,6 +1193,16 @@ static void addFortranMain(const ToolChain &TC, const ArgList &Args,
     return;
   }
 
+  const Driver &D = TC.getDriver();
+  const char *LinkFlag = "-lFortran_main";
+
+  // warn if -lFortran_main was already specified
+  for (const char *arg : CmdArgs) {
+    if (strncmp(arg, LinkFlag, strlen(LinkFlag)) == 0)
+      D.Diag(diag::warn_drv_deprecated_custom)
+          << LinkFlag << "see the flang driver documentation for correct usage";
+  }
+
   // 2. GNU and similar
   // The --whole-archive option needs to be part of the link line to make
   // sure that the main() function from Fortran_main.a is pulled in by the
@@ -1201,12 +1211,12 @@ static void addFortranMain(const ToolChain &TC, const ArgList &Args,
   if (!isWholeArchivePresent(Args) && !TC.getTriple().isMacOSX() &&
       !TC.getTriple().isOSAIX()) {
     CmdArgs.push_back("--whole-archive");
-    CmdArgs.push_back("-lFortran_main");
+    CmdArgs.push_back(LinkFlag);
     CmdArgs.push_back("--no-whole-archive");
     return;
   }
 
-  CmdArgs.push_back("-lFortran_main");
+  CmdArgs.push_back(LinkFlag);
 }
 
 /// Add Fortran runtime libs
diff --git a/flang/test/Driver/linker-flags.f90 b/flang/test/Driver/linker-flags.f90
index ea91946316cfaa6..341f491bd778fbc 100644
--- a/flang/test/Driver/linker-flags.f90
+++ b/flang/test/Driver/linker-flags.f90
@@ -11,6 +11,7 @@
 ! RUN: %flang -### --target=x86_64-unknown-dragonfly %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,UNIX
 ! RUN: %flang -### --target=x86_64-unknown-haiku %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,HAIKU
 ! RUN: %flang -### --target=x86_64-windows-gnu %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,MINGW
+! RUN: %flang -### --target=aarch64-unknown-linux-gnu %S/Inputs/hello.f90 -lFortran_main 2>&1 | FileCheck %s --check-prefixes=DEPRECATED
 
 ! NOTE: Clang's driver library, clangDriver, usually adds 'oldnames' on Windows,
 !       but it is not needed when compiling Fortran code and they might bring in
@@ -53,3 +54,6 @@
 ! MSVC-LABEL: link
 ! MSVC-SAME: /subsystem:console
 ! MSVC-SAME: "[[object_file]]"
+
+! Check that we warn when using -lFortran_main
+! DEPRECATED: warning: argument '-lFortran_main' is deprecated, see the flang driver documentation for correct usage [-Wdeprecated]

mjklemm · 2024-01-22T17:36:37Z

I'm OK with landing this for Linux only at this point.

mjklemm

LGTM

kkwli

LG. Thanks

banach-space

Intended to warn users of the 19.x release not to do this.

A better solution should be found for the 20.x release. See discussion in #78152.

Unfortunately there is no warning on Windows currently. I am rushing to get this landed before 19.x branches.

Did you mean:

Intended to warn users of the 18.x release not to do this.

A better solution should be found for the 19.x release. See discussion in #78152.

Unfortunately there is no warning on Windows currently. I am rushing to get this landed before 19.x branches.

LGTM modulo a few comments. I like this direction!

banach-space · 2024-01-22T18:56:58Z

clang/lib/Driver/ToolChains/CommonArgs.cpp

@@ -1193,6 +1193,16 @@ static void addFortranMain(const ToolChain &TC, const ArgList &Args,
    return;
  }

+  const Driver &D = TC.getDriver();
+  const char *LinkFlag = "-lFortran_main";


Suggested change

const char *LinkFlag = "-lFortran_main";

const char *FortranMainLinkFlag = "-lFortran_main";

banach-space · 2024-01-22T18:58:03Z

clang/lib/Driver/ToolChains/CommonArgs.cpp

+  const Driver &D = TC.getDriver();
+  const char *LinkFlag = "-lFortran_main";
+
+  // warn if -lFortran_main was already specified


Suggested change

// warn if -lFortran_main was already specified

// Warn if the user added `-lFortran_main` - this library is an implementation detail of Flang and should be handled automatically by the driver.

banach-space · 2024-01-22T18:58:14Z

clang/lib/Driver/ToolChains/CommonArgs.cpp

+  for (const char *arg : CmdArgs) {
+    if (strncmp(arg, LinkFlag, strlen(LinkFlag)) == 0)
+      D.Diag(diag::warn_drv_deprecated_custom)
+          << LinkFlag << "see the flang driver documentation for correct usage";


[nit]

Suggested change

<< LinkFlag << "see the flang driver documentation for correct usage";

<< LinkFlag << "see the Flang driver documentation for correct usage";

banach-space · 2024-01-22T19:03:23Z

clang/lib/Driver/ToolChains/CommonArgs.cpp

@@ -1193,6 +1193,16 @@ static void addFortranMain(const ToolChain &TC, const ArgList &Args,
    return;
  }

+  const Driver &D = TC.getDriver();


Move this whole block below the // 2. GNU and similar? Otherwise it seems that this is still part of MSVC processing.

tblah requested review from banach-space, kiranchandramohan and mjklemm January 22, 2024 17:33

llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" flang:driver flang Flang issues not falling into any other category labels Jan 22, 2024

mjklemm approved these changes Jan 22, 2024

View reviewed changes

kkwli approved these changes Jan 22, 2024

View reviewed changes

banach-space approved these changes Jan 22, 2024

View reviewed changes

Fix review comments

1ba503b

tblah merged commit d37d1c8 into llvm:main Jan 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[flang][driver] deprecate manual usage of -lFortran_main #79016

[flang][driver] deprecate manual usage of -lFortran_main #79016

Uh oh!

tblah commented Jan 22, 2024 •

edited

Loading

Uh oh!

llvmbot commented Jan 22, 2024 •

edited

Loading

Uh oh!

mjklemm commented Jan 22, 2024

Uh oh!

mjklemm left a comment

Uh oh!

kkwli left a comment

Uh oh!

banach-space left a comment

Uh oh!

banach-space Jan 22, 2024

Uh oh!

banach-space Jan 22, 2024

Uh oh!

banach-space Jan 22, 2024

Uh oh!

banach-space Jan 22, 2024

Uh oh!

Uh oh!

	const char *LinkFlag = "-lFortran_main";
	const char *FortranMainLinkFlag = "-lFortran_main";

	// warn if -lFortran_main was already specified
	// Warn if the user added `-lFortran_main` - this library is an implementation detail of Flang and should be handled automatically by the driver.

	<< LinkFlag << "see the flang driver documentation for correct usage";
	<< LinkFlag << "see the Flang driver documentation for correct usage";

[flang][driver] deprecate manual usage of -lFortran_main #79016

[flang][driver] deprecate manual usage of -lFortran_main #79016

Uh oh!

Conversation

tblah commented Jan 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jan 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mjklemm commented Jan 22, 2024

Uh oh!

mjklemm left a comment

Choose a reason for hiding this comment

Uh oh!

kkwli left a comment

Choose a reason for hiding this comment

Uh oh!

banach-space left a comment

Choose a reason for hiding this comment

Uh oh!

banach-space Jan 22, 2024

Choose a reason for hiding this comment

Uh oh!

banach-space Jan 22, 2024

Choose a reason for hiding this comment

Uh oh!

banach-space Jan 22, 2024

Choose a reason for hiding this comment

Uh oh!

banach-space Jan 22, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tblah commented Jan 22, 2024 •

edited

Loading

llvmbot commented Jan 22, 2024 •

edited

Loading