[flang][driver] Avoid mentions of Clang in Flang's command line reference. #88932

Meinersbur · 2024-04-16T17:01:02Z

The help text was not updated in #87360.

Clang is also mentioned for the diagnostic warnings reference, which mostly applies to C/C++/Obj-C, not Fortran. #81726 already tried to fix this, and I don't know a better solution.

The help text was not updated in llvm#87360. Clang is also mentioned for the diagnositic warnings reference, which mostly applies to C/C++/Obj-C, not Fortran. llvm#81726 already tried to fix this, and I don't know a better solution.

llvmbot · 2024-04-17T08:00:06Z

@llvm/pr-subscribers-clang

Author: Michael Kruse (Meinersbur)

Changes

The help text was not updated in #87360.

Clang is also mentioned for the diagnostic warnings reference, which mostly applies to C/C++/Obj-C, not Fortran. #81726 already tried to fix this, and I don't know a better solution.

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

3 Files Affected:

(modified) clang/include/clang/Driver/Options.td (+6-2)
(modified) flang/test/Driver/driver-help-hidden.f90 (+1-1)
(modified) flang/test/Driver/driver-help.f90 (+1-1)

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index e24626913add76..d89b53e4049cf6 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -807,8 +807,12 @@ def gcc_install_dir_EQ : Joined<["--"], "gcc-install-dir=">,
   "Note: executables (e.g. ld) used by the compiler are not overridden by the selected GCC installation">;
 def gcc_toolchain : Joined<["--"], "gcc-toolchain=">, Flags<[NoXarchOption]>,
   Visibility<[ClangOption, FlangOption]>,
-  HelpText<"Specify a directory where Clang can find 'include' and 'lib{,32,64}/gcc{,-cross}/$triple/$version'. "
-  "Clang will use the GCC installation with the largest version">;
+  HelpText<
+    "Specify a directory where Clang can find 'include' and 'lib{,32,64}/gcc{,-cross}/$triple/$version'. "
+    "Clang will use the GCC installation with the largest version">,
+  HelpTextForVariants<[FlangOption],
+    "Specify a directory where Flang can find 'lib{,32,64}/gcc{,-cross}/$triple/$version'. "
+    "Flang will use the GCC installation with the largest version">;
 def gcc_triple_EQ : Joined<["--"], "gcc-triple=">,
   HelpText<"Search for the GCC installation with the specified triple.">;
 def CC : Flag<["-"], "CC">, Visibility<[ClangOption, CC1Option]>,
diff --git a/flang/test/Driver/driver-help-hidden.f90 b/flang/test/Driver/driver-help-hidden.f90
index de2fe3048f993c..b5bb0f1c1b2560 100644
--- a/flang/test/Driver/driver-help-hidden.f90
+++ b/flang/test/Driver/driver-help-hidden.f90
@@ -109,7 +109,7 @@
 ! CHECK-NEXT: -fxor-operator          Enable .XOR. as a synonym of .NEQV.
 ! CHECK-NEXT: --gcc-install-dir=<value>
 ! CHECK-NEXT:                         Use GCC installation in the specified directory. The directory ends with path components like 'lib{,32,64}/gcc{,-cross}/$triple/$version'. Note: executables (e.g. ld) used by the compiler are not overridden by the selected GCC installation
-! CHECK-NEXT: --gcc-toolchain=<value> Specify a directory where Clang can find 'include' and 'lib{,32,64}/gcc{,-cross}/$triple/$version'. Clang will use the GCC installation with the largest version
+! CHECK-NEXT: --gcc-toolchain=<value> Specify a directory where Flang can find 'lib{,32,64}/gcc{,-cross}/$triple/$version'. Flang will use the GCC installation with the largest version
 ! CHECK-NEXT: -gline-directives-only  Emit debug line info directives only
 ! CHECK-NEXT: -gline-tables-only      Emit debug line number tables only
 ! CHECK-NEXT: -gpulibc                Link the LLVM C Library for GPUs
diff --git a/flang/test/Driver/driver-help.f90 b/flang/test/Driver/driver-help.f90
index b258eb59c18629..0b0a493baf07f7 100644
--- a/flang/test/Driver/driver-help.f90
+++ b/flang/test/Driver/driver-help.f90
@@ -97,7 +97,7 @@
 ! HELP-NEXT: -fxor-operator          Enable .XOR. as a synonym of .NEQV.
 ! HELP-NEXT: --gcc-install-dir=<value>
 ! HELP-NEXT:                         Use GCC installation in the specified directory. The directory ends with path components like 'lib{,32,64}/gcc{,-cross}/$triple/$version'. Note: executables (e.g. ld) used by the compiler are not overridden by the selected GCC installation
-! HELP-NEXT: --gcc-toolchain=<value> Specify a directory where Clang can find 'include' and 'lib{,32,64}/gcc{,-cross}/$triple/$version'. Clang will use the GCC installation with the largest version
+! HELP-NEXT: --gcc-toolchain=<value> Specify a directory where Flang can find 'lib{,32,64}/gcc{,-cross}/$triple/$version'. Flang will use the GCC installation with the largest version
 ! HELP-NEXT: -gline-directives-only  Emit debug line info directives only
 ! HELP-NEXT: -gline-tables-only      Emit debug line number tables only
 ! HELP-NEXT: -gpulibc                Link the LLVM C Library for GPUs

llvmbot · 2024-04-17T08:00:07Z

@llvm/pr-subscribers-flang-driver

Author: Michael Kruse (Meinersbur)

Changes

The help text was not updated in #87360.

Clang is also mentioned for the diagnostic warnings reference, which mostly applies to C/C++/Obj-C, not Fortran. #81726 already tried to fix this, and I don't know a better solution.

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

3 Files Affected:

(modified) clang/include/clang/Driver/Options.td (+6-2)
(modified) flang/test/Driver/driver-help-hidden.f90 (+1-1)
(modified) flang/test/Driver/driver-help.f90 (+1-1)

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index e24626913add76..d89b53e4049cf6 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -807,8 +807,12 @@ def gcc_install_dir_EQ : Joined<["--"], "gcc-install-dir=">,
   "Note: executables (e.g. ld) used by the compiler are not overridden by the selected GCC installation">;
 def gcc_toolchain : Joined<["--"], "gcc-toolchain=">, Flags<[NoXarchOption]>,
   Visibility<[ClangOption, FlangOption]>,
-  HelpText<"Specify a directory where Clang can find 'include' and 'lib{,32,64}/gcc{,-cross}/$triple/$version'. "
-  "Clang will use the GCC installation with the largest version">;
+  HelpText<
+    "Specify a directory where Clang can find 'include' and 'lib{,32,64}/gcc{,-cross}/$triple/$version'. "
+    "Clang will use the GCC installation with the largest version">,
+  HelpTextForVariants<[FlangOption],
+    "Specify a directory where Flang can find 'lib{,32,64}/gcc{,-cross}/$triple/$version'. "
+    "Flang will use the GCC installation with the largest version">;
 def gcc_triple_EQ : Joined<["--"], "gcc-triple=">,
   HelpText<"Search for the GCC installation with the specified triple.">;
 def CC : Flag<["-"], "CC">, Visibility<[ClangOption, CC1Option]>,
diff --git a/flang/test/Driver/driver-help-hidden.f90 b/flang/test/Driver/driver-help-hidden.f90
index de2fe3048f993c..b5bb0f1c1b2560 100644
--- a/flang/test/Driver/driver-help-hidden.f90
+++ b/flang/test/Driver/driver-help-hidden.f90
@@ -109,7 +109,7 @@
 ! CHECK-NEXT: -fxor-operator          Enable .XOR. as a synonym of .NEQV.
 ! CHECK-NEXT: --gcc-install-dir=<value>
 ! CHECK-NEXT:                         Use GCC installation in the specified directory. The directory ends with path components like 'lib{,32,64}/gcc{,-cross}/$triple/$version'. Note: executables (e.g. ld) used by the compiler are not overridden by the selected GCC installation
-! CHECK-NEXT: --gcc-toolchain=<value> Specify a directory where Clang can find 'include' and 'lib{,32,64}/gcc{,-cross}/$triple/$version'. Clang will use the GCC installation with the largest version
+! CHECK-NEXT: --gcc-toolchain=<value> Specify a directory where Flang can find 'lib{,32,64}/gcc{,-cross}/$triple/$version'. Flang will use the GCC installation with the largest version
 ! CHECK-NEXT: -gline-directives-only  Emit debug line info directives only
 ! CHECK-NEXT: -gline-tables-only      Emit debug line number tables only
 ! CHECK-NEXT: -gpulibc                Link the LLVM C Library for GPUs
diff --git a/flang/test/Driver/driver-help.f90 b/flang/test/Driver/driver-help.f90
index b258eb59c18629..0b0a493baf07f7 100644
--- a/flang/test/Driver/driver-help.f90
+++ b/flang/test/Driver/driver-help.f90
@@ -97,7 +97,7 @@
 ! HELP-NEXT: -fxor-operator          Enable .XOR. as a synonym of .NEQV.
 ! HELP-NEXT: --gcc-install-dir=<value>
 ! HELP-NEXT:                         Use GCC installation in the specified directory. The directory ends with path components like 'lib{,32,64}/gcc{,-cross}/$triple/$version'. Note: executables (e.g. ld) used by the compiler are not overridden by the selected GCC installation
-! HELP-NEXT: --gcc-toolchain=<value> Specify a directory where Clang can find 'include' and 'lib{,32,64}/gcc{,-cross}/$triple/$version'. Clang will use the GCC installation with the largest version
+! HELP-NEXT: --gcc-toolchain=<value> Specify a directory where Flang can find 'lib{,32,64}/gcc{,-cross}/$triple/$version'. Flang will use the GCC installation with the largest version
 ! HELP-NEXT: -gline-directives-only  Emit debug line info directives only
 ! HELP-NEXT: -gline-tables-only      Emit debug line number tables only
 ! HELP-NEXT: -gpulibc                Link the LLVM C Library for GPUs

banach-space

Nice, thank you! LGTM

clang/include/clang/Driver/Options.td

DavidSpickett · 2024-04-17T08:11:55Z

Clang is also mentioned for the diagnostic warnings reference, which mostly applies to C/C++/Obj-C, not Fortran. #81726 already tried to fix this, and I don't know a better solution.

Do you mean that this PR fixes this, or that you noticed this problem while working on this?

If it's the latter it may be as @banach-space mentioned to me, that the docbrief is still shared between Flang and Clang. This could be solved in a similar, albeit tedious way to what I did for the help text.

banach-space · 2024-04-17T08:45:43Z

Clang is also mentioned for the diagnostic warnings reference, which mostly applies to C/C++/Obj-C, not Fortran. #81726 already tried to fix this, and I don't know a better solution.

Do you mean that this PR fixes this, or that you noticed this problem while working on this?

If it's the latter it may be as @banach-space mentioned to me, that the docbrief is still shared between Flang and Clang. This could be solved in a similar, albeit tedious way to what I did for the help text.

@Meinersbur Could you share an example? If it's a problem specific to diagnostics then we should possibly just start separating Clang and Flang diagnostics. That could mean a bit of duplication, but not too much, so I'm not worried.

Meinersbur · 2024-04-17T09:05:54Z

Do you mean that this PR fixes this, or that you noticed this problem while working on this?

I noticed this when working on the patch, .i.e that https://flang.llvm.org/docs/FlangCommandLineReference.html still mentioned Clang in several places and since it was common, I didn't want to fix it only for --gcc-toolchain. HelpTextForVariants was introduced later, reducing the number of "Clang" in FlangCommandLineReference already. This fixes --gcc-toolchain, but there is no list of warnings supported by Flang, so I don't have anything beyond what #81726 already did.

If it's the latter it may be as @banach-space mentioned to me, that the docbrief is still shared between Flang and Clang. This could be solved in a similar, albeit tedious way to what I did for the help text.

The docbrief strings already use %Program/GlobalDocumentation.Program which seems to work and made me think that HelpTextForVariants was actually unnecessary. Would you like me to introduce DocBriefForVariants?

@Meinersbur Could you share an example? If it's a problem specific to diagnostics then we should possibly just start separating Clang and Flang diagnostics. That could mean a bit of duplication, but not too much, so I'm not worried.

def Diag_Group : OptionGroup<"<W/R group>">, Group<CompileOnly_Group>,
                 DocName<"Diagnostic options">,
                 DocBrief<!strconcat(
                   StringForProgram<"Flags controlling which warnings, errors, and remarks %Program will generate. ">.str,
                   !cond(!eq(GlobalDocumentation.Program, "Clang"):
                       "See the :doc:`full list of warning and remark flags <DiagnosticsReference>`.",
                     true:
                       "See Clang's Diagnostic Reference for a full list of warning and remark flags."
                   )
                 )>;

DavidSpickett · 2024-04-17T09:47:22Z

The docbrief strings already use %Program/GlobalDocumentation.Program which seems to work and made me think that HelpTextForVariants was actually unnecessary. Would you like me to introduce DocBriefForVariants?

IIRC it was unnecessary for the documentation because it gets built multiple times, for the driver it's built once so it has to include all possible variants the first time.

I have no preference for how it's done, if the %Program thing works then fine. We only need a ForVariants if it's something people see when interacting with Flang itself I think.

DavidSpickett

The changes themselves LGTM, thanks for fixing this.

banach-space · 2024-04-17T13:54:27Z

Would you like me to introduce DocBriefForVariants?

+1 That would be helpful for -I:

https://flang.llvm.org/docs/FlangCommandLineReference.html#cmdoption-flang-I-dir

Ideally we'd find more examples (so that you are not adding it for just one option).

As for this:

def Diag_Group : OptionGroup<"<W/R group>">, Group<CompileOnly_Group>,
                 DocName<"Diagnostic options">,
                 DocBrief<!strconcat(
                   StringForProgram<"Flags controlling which warnings, errors, and remarks %Program will generate. ">.str,
                   !cond(!eq(GlobalDocumentation.Program, "Clang"):
                       "See the :doc:`full list of warning and remark flags <DiagnosticsReference>`.",
                     true:
                       "See Clang's Diagnostic Reference for a full list of warning and remark flags."
                   )
                 )>;

Wouldn't replacing Clang's with %Program's be sufficient in this case?

tblah

The changes in this PR look good to me. Thanks for the fix!

Meinersbur · 2024-04-22T09:55:36Z

IIRC it was unnecessary for the documentation because it gets built multiple times, for the driver it's built once so it has to include all possible variants the first time.

I have no preference for how it's done, if the %Program thing works then fine. We only need a ForVariants if it's something people see when interacting with Flang itself I think.

I didn't know this and experimented a bit. Indeed %Program in HelpText gets emitted as "Clang" in flang-new --help. DocBrief and HelpText are "Flang" for FlangCommandLineReference.rst.

So DocBriefForVariant would not add new functionality, but might be useful so DocBrief[ForVariant] and HelpText[ForVariant] can use the same syntax and avoid confusion. I will merge this patch and work on DocBriefForVariant after that.

Meinersbur changed the title ~~Avoid mentions of Clang in Flang's command line reference.~~ [flang][driver] Avoid mentions of Clang in Flang's command line reference. Apr 16, 2024

Meinersbur marked this pull request as ready for review April 17, 2024 07:59

llvmbot added clang Clang issues not falling into any other category flang:driver flang Flang issues not falling into any other category labels Apr 17, 2024

Meinersbur requested review from banach-space, kiranchandramohan and tblah April 17, 2024 08:00

banach-space approved these changes Apr 17, 2024

View reviewed changes

Meinersbur requested review from DavidSpickett, banach-space and luporl April 17, 2024 08:05

DavidSpickett reviewed Apr 17, 2024

View reviewed changes

clang/include/clang/Driver/Options.td Show resolved Hide resolved

DavidSpickett approved these changes Apr 17, 2024

View reviewed changes

tblah approved these changes Apr 17, 2024

View reviewed changes

banach-space approved these changes Apr 18, 2024

View reviewed changes

luporl approved these changes Apr 19, 2024

View reviewed changes

Meinersbur merged commit fdc8c54 into llvm:main Apr 22, 2024

This was referenced Apr 22, 2024

[libc] Make fenv and math tests preserve fenv_t state #89658

Merged

[libc] Clean up alternate test framework support #89659

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[flang][driver] Avoid mentions of Clang in Flang's command line reference. #88932

[flang][driver] Avoid mentions of Clang in Flang's command line reference. #88932

Uh oh!

Meinersbur commented Apr 16, 2024

Uh oh!

llvmbot commented Apr 17, 2024

Uh oh!

llvmbot commented Apr 17, 2024

Uh oh!

banach-space left a comment

Uh oh!

Uh oh!

DavidSpickett commented Apr 17, 2024

Uh oh!

banach-space commented Apr 17, 2024

Uh oh!

Meinersbur commented Apr 17, 2024

Uh oh!

DavidSpickett commented Apr 17, 2024 •

edited

Loading

Uh oh!

DavidSpickett left a comment

Uh oh!

banach-space commented Apr 17, 2024

Uh oh!

tblah left a comment

Uh oh!

Meinersbur commented Apr 22, 2024

Uh oh!

Uh oh!

[flang][driver] Avoid mentions of Clang in Flang's command line reference. #88932

[flang][driver] Avoid mentions of Clang in Flang's command line reference. #88932

Uh oh!

Conversation

Meinersbur commented Apr 16, 2024

Uh oh!

llvmbot commented Apr 17, 2024

Uh oh!

llvmbot commented Apr 17, 2024

Uh oh!

banach-space left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DavidSpickett commented Apr 17, 2024

Uh oh!

banach-space commented Apr 17, 2024

Uh oh!

Meinersbur commented Apr 17, 2024

Uh oh!

DavidSpickett commented Apr 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DavidSpickett left a comment

Choose a reason for hiding this comment

Uh oh!

banach-space commented Apr 17, 2024

Uh oh!

tblah left a comment

Choose a reason for hiding this comment

Uh oh!

Meinersbur commented Apr 22, 2024

Uh oh!

Uh oh!

DavidSpickett commented Apr 17, 2024 •

edited

Loading