[clang-tblgen] Automatically document options values

[serge-sans-paille]

Summary

This is a port of f5c666742f7bb4ae79ec79c8acf61dced4d37cc9 to clang's tablegen.
The code duplication is a bit sad there :-/

Related to https://reviews.llvm.org/D122378

[MaskRay]

Thank you for improving the tool :)

cd clang
path/to/clang-tblgen --gen-opt-docs -I ../llvm/include -I include/clang/Driver include/clang/Driver/ClangOptionDocs.td > /tmp/0
nvim -d docs/ClangCommandLineReference.rst /tmp/0

Hope a native speaker (@aaron.ballman @dexonsmith @jhenderson ) can suggest the usage here.

For an option with more than 2 choices: the current documentation is <arg> can be one of 'return', 'branch', 'full' or 'none',
I'm thinking of: <arg> should be 'return', 'branch', 'full', or 'none'

When there are two choices (-gsplit-dwarf=<arg>), currently the documentation is <arg> can be one of 'split' or 'single'.
I am thinking of <arg> should be 'split' or 'single'.

clang/utils/TableGen/ClangOptionDocEmitter.cpp

241

constexpr StringLiteral

const variable at a namespace scope is automatically internal linkage, except some weird corner cases.

[aaron.ballman]

In D123682#3450215, @MaskRay wrote:

Thank you for improving the tool :)

cd clang
path/to/clang-tblgen --gen-opt-docs -I ../llvm/include -I include/clang/Driver include/clang/Driver/ClangOptionDocs.td > /tmp/0
nvim -d docs/ClangCommandLineReference.rst /tmp/0

Hope a native speaker (@aaron.ballman @dexonsmith @jhenderson ) can suggest the usage here.

For an option with more than 2 choices: the current documentation is <arg> can be one of 'return', 'branch', 'full' or 'none',
I'm thinking of: <arg> should be 'return', 'branch', 'full', or 'none'

When there are two choices (-gsplit-dwarf=<arg>), currently the documentation is <arg> can be one of 'split' or 'single'.
I am thinking of <arg> should be 'split' or 'single'.

Both of these suggestions seem reasonable to me (shorter but equally as clear as before), but we should fix to be consistent in llvm/utils/TableGen/OptRSTEmitter.cpp if we opt to go this route.

The changes here LGTM as-is (I'm happy with either current wording or the changed wording). Thanks for this!

[serge-sans-paille]

@aaron.ballman Any thoughs on the above suggestion? I'd be happy to adopt any of those :-)

[aaron.ballman]

In D123682#3454627, @serge-sans-paille wrote:

@aaron.ballman Any thoughs on the above suggestion? I'd be happy to adopt any of those :-)

I'd go with:

More than two choices: <arg> should be 'return', 'branch', 'full', or 'none'
Only two choices: <arg> should be 'split' or 'single'.

[jhenderson]

In D123682#3455793, @aaron.ballman wrote:

In D123682#3454627, @serge-sans-paille wrote:

@aaron.ballman Any thoughs on the above suggestion? I'd be happy to adopt any of those :-)

I'd go with:

More than two choices: <arg> should be 'return', 'branch', 'full', or 'none'
Only two choices: <arg> should be 'split' or 'single'.

FWIW, I'd actually use "must" rather than "should", but otherwise I agree with this. "should" implies there are cases where it is okay to use a different value, which is obviously not the intent.

[serge-sans-paille]

Adopt a better wording.

[aaron.ballman]

LGTM!

clang/utils/TableGen/ClangOptionDocEmitter.cpp

void emitOptionWithArgs(StringRef Prefix, const Record *Option,
 
   StringRef Separator = Separators.first;
   for (auto Arg : Args) {
     OS << Separator << escapeRST(Arg);
     Separator = Separators.second;
   }
 }
 
+constexpr StringLiteral DefaultMetaVarName = "<arg>";
+
 void emitOptionName(StringRef Prefix, const Record *Option, raw_ostream &OS) {
   // Find the arguments to list after the option.
   unsigned NumArgs = getNumArgsForKind(Option->getValueAsDef("Kind"), Option);
   bool HasMetaVarName = !Option->isValueUnset("MetaVarName");
 
   std::vector<std::string> Args;
   if (HasMetaVarName)
     Args.push_back(std::string(Option->getValueAsString("MetaVarName")));
   else if (NumArgs == 1)
-    Args.push_back("<arg>");
+    Args.push_back(DefaultMetaVarName.str());
 
   // Fill up arguments if this option didn't provide a meta var name or it
   // supports an unlimited number of arguments. We can't see how many arguments
   // already are in a meta var name, so assume it has right number. This is
   // needed for JoinedAndSeparate options so that there arent't too many
   // arguments.
   if (!HasMetaVarName || NumArgs == UnlimitedArgs) {
     while (Args.size() < NumArgs) {
void emitOption(const DocumentedOption &Option, const Record *DocInfo,
   forEachOptionName(Option, DocInfo, [&](const Record *Option) {
     EmittedAny = emitOptionNames(Option, OS, EmittedAny);
   });
   if (SphinxWorkaroundSuffix)
     OS << "\n.. program:: " << DocInfo->getValueAsString("Program");
   OS << "\n\n";
 
   // Emit the description, if we have one.
+  const Record *R = Option.Option;
   std::string Description =
-      getRSTStringWithTextFallback(Option.Option, "DocBrief", "HelpText");
+      getRSTStringWithTextFallback(R, "DocBrief", "HelpText");
+
+  if (!isa<UnsetInit>(R->getValueInit("Values"))) {
+    if (!Description.empty() && Description.back() != '.')
+      Description.push_back('.');
+
+    StringRef MetaVarName;
+    if (!isa<UnsetInit>(R->getValueInit("MetaVarName")))
+      MetaVarName = R->getValueAsString("MetaVarName");
+    else
+      MetaVarName = DefaultMetaVarName;
+
+    SmallVector<StringRef> Values;
+    SplitString(R->getValueAsString("Values"), Values, ",");
+    Description += (" " + MetaVarName + " must be '").str();
+    if (Values.size() > 1) {
+      Description += join(Values.begin(), Values.end() - 1, "', '");
+      Description += "' or '";
+    }
+    Description += (Values.back() + "'.").str();
+  }
+
   if (!Description.empty())
     OS << Description << "\n\n";
 }
 
 void emitDocumentation(int Depth, const Documentation &Doc,
                        const Record *DocInfo, raw_ostream &OS);
 
 void emitGroup(int Depth, const DocumentedGroup &Group, const Record *DocInfo,

llvm/utils/TableGen/OptRSTEmitter.cpp

for (Record *R : KV.getValue()) {
         HelpText = R->getValueAsString("HelpText").trim().str();
         if (!HelpText.empty() && HelpText.back() != '.')
           HelpText.push_back('.');
       }
 
       if (!isa<UnsetInit>(R->getValueInit("Values"))) {
         SmallVector<StringRef> Values;
         SplitString(R->getValueAsString("Values"), Values, ",");
-        HelpText += (" " + MetaVarName + " can be ").str();
+        HelpText += (" " + MetaVarName + " must be '").str();
 
-        if (Values.size() == 1) {
-          HelpText += ("'" + Values.front() + "'.").str();
-        } else {
-          HelpText += "one of '";
+        if (Values.size() > 1) {
           HelpText += join(Values.begin(), Values.end() - 1, "', '");
-          HelpText += ("' or '" + Values.back() + "'.").str();
+          HelpText += "' or '";
         }
+        HelpText += (Values.front() + "'.").str();
       }
 
       if (!HelpText.empty()) {
         OS << ' ';
         OS.write_escaped(HelpText);
         OS << "\n\n";
       }
     }
   }
 }
 } // end namespace llvm