[clang][flang][driver] Correct program names in option group descriptions #81726
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: David Spickett (DavidSpickett) ChangesCurrently https://flang.llvm.org/docs/FlangCommandLineReference.html refers to "Clang" in several of the group descriptions for example: This is pretty confusing. I'm fixing this by making use of This To make sure the file is valid when not generating docs, I'm checking a #define and providing a default GlobalDocumentation if it's not defined. (I looked for a way to check if a def exists, but tablegen doesn't seem to have one) This means that if the DocBrief are used outside of documentation, they'll say "Clang", which is the same as it always was. This change does not aim fix option descriptions that refer to clang. Though we can use parts of this for that, there is only one driver library so it needs a different approach. Full diff: https://github.com/llvm/llvm-project/pull/81726.diff 4 Files Affected:
diff --git a/clang/include/clang/Driver/ClangOptionDocs.td b/clang/include/clang/Driver/ClangOptionDocs.td
index a5ee577c5f45db..dea6a7ccb12c9a 100644
--- a/clang/include/clang/Driver/ClangOptionDocs.td
+++ b/clang/include/clang/Driver/ClangOptionDocs.td
@@ -27,11 +27,12 @@ GCC-compatible ``clang`` and ``clang++`` drivers.
}];
- string Program = "clang";
+ string Program = "Clang";
// Note: We *must* use DefaultVis and not ClangOption, since that's
// the name of the actual TableGen record. The alias will not work.
list<string> VisibilityMask = ["DefaultVis"];
list<string> IgnoreFlags = ["HelpHidden", "Unsupported", "Ignored"];
}
+#define GENERATING_DOCS
include "Options.td"
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 95b464e7d61834..375ea6f5f6979c 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -13,6 +13,25 @@
// Include the common option parsing interfaces.
include "llvm/Option/OptParser.td"
+// When generating documentation, we expect there to be a GlobalDocumentation
+// def containing the program name that we are generating documentation for.
+// This object should only be used by things that are used in documentation,
+// such as the group descriptions.
+#ifndef GENERATING_DOCS
+// So that this file can still be parsed without such a def, define one if there
+// isn't one provided.
+def GlobalDocumentation {
+ // Sensible default in case of mistakes.
+ string Program = "Clang";
+}
+#endif
+
+// Use this to generate program specific documentation, for example:
+// StringForProgram<"Control how %Program behaves.">.str
+class StringForProgram<string _str> {
+ string str = !subst("%Program", GlobalDocumentation.Program, _str);
+}
+
/////////
// Flags
@@ -100,14 +119,16 @@ def Action_Group : OptionGroup<"<action group>">, DocName<"Actions">,
// Meta-group for options which are only used for compilation,
// and not linking etc.
def CompileOnly_Group : OptionGroup<"<CompileOnly group>">,
- DocName<"Compilation options">, DocBrief<[{
-Flags controlling the behavior of Clang during compilation. These flags have
-no effect during actions that do not perform compilation.}]>;
+ DocName<"Compilation options">,
+ DocBrief<StringForProgram<[{
+Flags controlling the behavior of %Program during compilation. These flags have
+no effect during actions that do not perform compilation.}]>.str>;
def Preprocessor_Group : OptionGroup<"<Preprocessor group>">,
Group<CompileOnly_Group>,
- DocName<"Preprocessor options">, DocBrief<[{
-Flags controlling the behavior of the Clang preprocessor.}]>;
+ DocName<"Preprocessor options">,
+ DocBrief<StringForProgram<[{
+Flags controlling the behavior of the %Program preprocessor.}]>.str>;
def IncludePath_Group : OptionGroup<"<I/i group>">, Group<Preprocessor_Group>,
DocName<"Include path management">,
@@ -128,9 +149,9 @@ def d_Group : OptionGroup<"<d group>">, Group<Preprocessor_Group>,
Flags allowing the state of the preprocessor to be dumped in various ways.}]>;
def Diag_Group : OptionGroup<"<W/R group>">, Group<CompileOnly_Group>,
- DocName<"Diagnostic options">, DocBrief<[{
-Flags controlling which warnings, errors, and remarks Clang will generate.
-See the :doc:`full list of warning and remark flags <DiagnosticsReference>`.}]>;
+ DocName<"Diagnostic options">, DocBrief<StringForProgram<[{
+Flags controlling which warnings, errors, and remarks %Program will generate.
+See the :doc:`full list of warning and remark flags <DiagnosticsReference>`.}]>.str>;
def R_Group : OptionGroup<"<R group>">, Group<Diag_Group>, DocFlatten;
def R_value_Group : OptionGroup<"<R (with value) group>">, Group<R_Group>,
diff --git a/clang/utils/TableGen/ClangOptionDocEmitter.cpp b/clang/utils/TableGen/ClangOptionDocEmitter.cpp
index a4095950ca975f..3fe98909940749 100644
--- a/clang/utils/TableGen/ClangOptionDocEmitter.cpp
+++ b/clang/utils/TableGen/ClangOptionDocEmitter.cpp
@@ -342,9 +342,10 @@ void emitOption(const DocumentedOption &Option, const Record *DocInfo,
})];
for (auto &S : SphinxOptionIDs)
NextSuffix[S] = SphinxWorkaroundSuffix + 1;
+
+ std::string Program = DocInfo->getValueAsString("Program").lower();
if (SphinxWorkaroundSuffix)
- OS << ".. program:: " << DocInfo->getValueAsString("Program")
- << SphinxWorkaroundSuffix << "\n";
+ OS << ".. program:: " << Program << SphinxWorkaroundSuffix << "\n";
// Emit the names of the option.
OS << ".. option:: ";
@@ -353,7 +354,7 @@ void emitOption(const DocumentedOption &Option, const Record *DocInfo,
EmittedAny = emitOptionNames(Option, OS, EmittedAny);
});
if (SphinxWorkaroundSuffix)
- OS << "\n.. program:: " << DocInfo->getValueAsString("Program");
+ OS << "\n.. program:: " << Program;
OS << "\n\n";
// Emit the description, if we have one.
@@ -421,7 +422,7 @@ void clang::EmitClangOptDocs(RecordKeeper &Records, raw_ostream &OS) {
return;
}
OS << DocInfo->getValueAsString("Intro") << "\n";
- OS << ".. program:: " << DocInfo->getValueAsString("Program") << "\n";
+ OS << ".. program:: " << DocInfo->getValueAsString("Program").lower() << "\n";
emitDocumentation(0, extractDocumentation(Records, DocInfo), DocInfo, OS);
}
diff --git a/flang/docs/FlangOptionsDocs.td b/flang/docs/FlangOptionsDocs.td
index 9189899e82c62d..14d033f8587e3b 100644
--- a/flang/docs/FlangOptionsDocs.td
+++ b/flang/docs/FlangOptionsDocs.td
@@ -24,10 +24,10 @@ Introduction
}];
- string Program = "flang";
+ string Program = "Flang";
list<string> VisibilityMask = ["FlangOption"];
list<string> IgnoreFlags = ["HelpHidden", "Unsupported", "Ignored"];
}
-
+#define GENERATING_DOCS
include "Options.td"
|
|
To add program specific option help, my idea is to store mutliple strings in the driver and select based on who's asking for the help. I think that warrants its own change so I'm not attempting that here. Also, there's only 2 flang options that refer to clang so it's not the top priority. |
|
@llvm/pr-subscribers-flang-driver |
|
Unrelated, but just wanted to bring this up since you are working in this area and distinguishing between Flang and Clang. Flang does not generate the Diagnostics Reference and hence ends up with the warning (see below) in the docs CI (https://lab.llvm.org/buildbot/#/builders/89/builds/57357). Is it possible to conditionally mention this in the FlangCommandLineReference? /home/buildbot/as-worker-4/publish-sphinx-docs/build/tools/flang/docs/Source/FlangCommandLineReference.rst:194: WARNING: unknown document: 'DiagnosticsReference' |
|
Yes I had to disable Sphinx's -Werr for that, I will try to fix it. |
|
Fixed the warning. |
…ions Currently https://flang.llvm.org/docs/FlangCommandLineReference.html refers to "Clang" in several of the group descriptions for example: ``` Compilation options Flags controlling the behavior of Clang during compilation... ``` This is pretty confusing. I'm fixing this by making use of `Program` from the existing GlobalDocumentation object to substitute in the program name to these descriptions. This `Program` has been changed to a proper noun given that it's easier to lower case a string than capitalise one character (syntax wise). The tablegen backend has been changed to lower it so that links in the RST/HTML remain the same as they were before. To make sure the file is valid when not generating docs, I'm checking a #define and providing a default GlobalDocumentation if it's not defined. (I looked for a way to check if a def exists, but tablegen doesn't seem to have one) This means that if the DocBrief are used outside of documentation, they'll say "Clang", which is the same as it always was. This change does not aim fix option descriptions that refer to clang. Though we can use parts of this for that, there is only one driver library so it needs a different approach.
DavidSpickett
force-pushed
the
driver-docs
branch
from
caf4b39
to
f948cb2
Compare
kiranchandramohan
approved these changes
Feb 14, 2024
banach-space
approved these changes
Feb 15, 2024
Meinersbur
added a commit
to Meinersbur/llvm-project
that referenced
this pull request
Apr 16, 2024
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently https://flang.llvm.org/docs/FlangCommandLineReference.html refers to "Clang" in several of the group descriptions for example:
This is pretty confusing. I'm fixing this by making use of
Programfrom the existing GlobalDocumentation object to substitute in the program name to these descriptions.This
Programhas been changed to a proper noun given that it's easier to lower case a string than capitalise one character (syntax wise). The tablegen backend has been changed to lower it so that links in the RST/HTML remain the same as they were before.To make sure the file is valid when not generating docs, I'm checking a #define and providing a default GlobalDocumentation if it's not defined. (I looked for a way to check if a def exists, but tablegen doesn't seem to have one)
This means that if the DocBrief are used outside of documentation, they'll say "Clang", which is the same as it always was.
This change does not aim fix option descriptions that refer to clang. Though we can use parts of this for that, there is only one driver library so it needs a different approach.
This change also fixes the warning:
Which is due to flang docs trying to link to clang docs. Now it will just tell the reader to go to Clang's page, which is not ideal but it is easy to find with Google at least.