[doc] Improve clang auto-generated help

[serge-sans-paille]

Summary

If no MetaVar is provided and an enumeration of values is set, document it in
the same way Python argparse does. Also add a proper unittest for help strings

This is a follow-up to 6d7317894f89cea29a2ecf7d86ac88adc40a5eaa

[serge-sans-paille]

@aaron.ballman : soft and gentle ping :-)

[aaron.ballman]

Generally looks reasonable to me, but it looks like Precommit CI caught some valid failures that need to be fixed:

Failed Tests (2):

Flang :: Driver/driver-help-hidden.f90
Flang :: Driver/driver-help.f90

llvm/lib/Option/OptTable.cpp

574–576

Use a Twine?

[serge-sans-paille]

Take review into account

[awarzynski]

Hi @serge-sans-paille, I just saw your comment on Discourse. Looking specifically at:

! HELP-FC1-NEXT: -std={c89,c90,iso9899:1990,iso9899:199409,gnu89,gnu90,c99,iso9899:1999,c9x,iso9899:199x,gnu99,gnu9x,c11,iso9899:2011,c1x,iso9899:201x,gnu11,gnu1x,c17,iso9899:2017,c18,iso9899:2018,gnu17,gnu18,c2x,gnu2x,c++98,c++03,gnu++98,gnu++03,c++11,c++0x,gnu++11,gnu++0x,c++14,c++1y,gnu++14,gnu++1y,c++17,c++1z,gnu++17,gnu++1z,c++20,c++2a,gnu++20,gnu++2a,c++2b,gnu++2b,cl1.0,cl,cl1.1,cl1.2,cl2.0,cl3.0,clc++1.0,clc++,clc++2021,CL,CL1.1,CL1.2,CL2.0,CL3.0,CLC++,CLC++1.0,CLC++2021,cuda,hip,hlsl,hlsl2015,hlsl2016,hlsl2017,hlsl2018,hlsl2021,hlsl202x,}

I think that listing the available options for -std= is helpful, but I'm not sure whether clang -help is the right place. It's already quite cluttered and impossible to navigate without piping to e.g. grep. In the past, we tried using DocBrief in Options.td for longer option comments. That wouldn't change the contents of clang -help, but IIRC, would be included in https://clang.llvm.org/docs/ClangCommandLineReference.html. In general, I think that it would be great to re-factor and organise clang -help a bit, but that would be a bigger job.

On a different note, we should definitely avoiding making flang-new display various C or C++ standards as available options for -std=<val>. Basically, -std=c++98 (and other similar options) are not supported by flang-new and should not be printed in flang-new -help.

HTH,
Andrzej

[serge-sans-paille]

Thanks for jumping in! This looks like a design error to me. The Values field is already used for code completion, so the behavior is already flawed... Whould it make sense to have a common file for shared options, and then a clang-specific and a flang specifc option for options with different values / meaning etc ?

[awarzynski]

In D122654#3454126, @serge-sans-paille wrote:

Thanks for jumping in!

Np! And apologies for going radio silent for so long, completely lost track of time!

This looks like a design error to me. The Values field is already used for code completion, so the behavior is already flawed...

Hm, why wouldn't you want to auto-complete with possible values too? For example, -std=c<tab> will only work if possible values are also used in auto-completion.

Whould it make sense to have a common file for shared options, and then a clang-specific and a flang specifc option for options with different values / meaning etc ?

Yes! :) Many options do/will have identical meaning in both frontends. But I agree that there should be a dedicated file for every driver:

• clang (Clang's compiler driver)
• clang -cc1 (Clang's frontend driver)
• clang -cc1as (Clang's integrated assembler driver)
• flang (Flang's compiler driver)
• flang-new -fc1 (Flang's frontend driver)
• <anything that I've missed>

Perhaps this is more extreme than what you had in mind? Shared options (with semantics that don't differ between drivers) could be kept in some common *.td file.

In general, IMHO Options.td is a good example of code re-use not being such a good thing :) @ekieri made some nice improvements there recently: https://reviews.llvm.org/D123070. And I discussed this briefly in the past: https://lists.llvm.org/pipermail/cfe-dev/2020-July/066393.html too.

flang/lib/Lower/IntrinsicCall.cpp

 
 /// Table that drives the fir generation depending on the intrinsic.
 /// one to one mapping with Fortran arguments. If no mapping is
 /// defined here for a generic intrinsic, genRuntimeCall will be called
 /// to look for a match in the runtime a emit a call. Note that the argument
 /// lowering rules for an intrinsic need to be provided only if at least one
 /// argument must not be lowered by value. In which case, the lowering rules
 /// should be provided for all the intrinsic arguments for completeness.
-static constexpr IntrinsicHandler handlers[]{
+static const IntrinsicHandler handlers[]{
     {"abs", &I::genAbs},
     {"achar", &I::genChar},
     {"adjustl",
      &I::genAdjustRtCall<fir::runtime::genAdjustL>,
      {{{"string", asAddr}}},
      /*isElemental=*/true},
     {"adjustr",
      &I::genAdjustRtCall<fir::runtime::genAdjustR>,

flang/test/Driver/driver-help-hidden.f90

 ! CHECK-NEXT: -mllvm <value>         Additional arguments to forward to LLVM's option processing
 ! CHECK-NEXT: -module-dir <dir>      Put MODULE files in <dir>
 ! CHECK-NEXT: -nocpp                 Disable predefined and command line preprocessor macros
 ! CHECK-NEXT: -o <file> Write output to <file>
 ! CHECK-NEXT: -pedantic              Warn on language extensions
 ! CHECK-NEXT: -print-effective-triple Print the effective target triple
 ! CHECK-NEXT: -print-target-triple    Print the normalized target triple
 ! CHECK-NEXT: -P                     Disable linemarker output in -E mode
-! CHECK-NEXT: -std=<value>           Language standard to compile for
+! CHECK-NEXT: -std={c89,c90,iso9899:1990,iso9899:199409,gnu89,gnu90,c99,iso9899:1999,c9x,iso9899:199x,gnu99,gnu9x,c11,iso9899:2011,c1x,iso9899:201x,gnu11,gnu1x,c17,iso9899:2017,c18,iso9899:2018,gnu17,gnu18,c2x,gnu2x,c++98,c++03,gnu++98,gnu++03,c++11,c++0x,gnu++11,gnu++0x,c++14,c++1y,gnu++14,gnu++1y,c++17,c++1z,gnu++17,gnu++1z,c++20,c++2a,gnu++20,gnu++2a,c++2b,gnu++2b,cl1.0,cl,cl1.1,cl1.2,cl2.0,cl3.0,clc++1.0,clc++,clc++2021,CL,CL1.1,CL1.2,CL2.0,CL3.0,CLC++,CLC++1.0,CLC++2021,cuda,hip,hlsl,hlsl2015,hlsl2016,hlsl2017,hlsl2018,hlsl2021,hlsl202x,}
+! CHECK-NEXT:  Language standard to compile for
 ! CHECK-NEXT: -S                     Only run preprocess and compilation steps
 ! CHECK-NEXT: --target=<value>        Generate code for the given target
 ! CHECK-NEXT: -U <macro>             Undefine macro <macro>
 ! CHECK-NEXT: --version Print version information
 ! CHECK-NEXT: -W<warning>            Enable the specified warning
 ! CHECK-NEXT: -Xflang <arg>          Pass <arg> to the flang compiler
 
 !-------------------------------------------------------------

flang/test/Driver/driver-help.f90

 ! HELP-NEXT: -mllvm <value>         Additional arguments to forward to LLVM's option processing
 ! HELP-NEXT: -module-dir <dir>      Put MODULE files in <dir>
 ! HELP-NEXT: -nocpp                 Disable predefined and command line preprocessor macros
 ! HELP-NEXT: -o <file>              Write output to <file>
 ! HELP-NEXT: -pedantic              Warn on language extensions
 ! HELP-NEXT: -print-effective-triple Print the effective target triple
 ! HELP-NEXT: -print-target-triple    Print the normalized target triple
 ! HELP-NEXT: -P                     Disable linemarker output in -E mode
-! HELP-NEXT: -std=<value>           Language standard to compile for
+! HELP-NEXT: -std={c89,c90,iso9899:1990,iso9899:199409,gnu89,gnu90,c99,iso9899:1999,c9x,iso9899:199x,gnu99,gnu9x,c11,iso9899:2011,c1x,iso9899:201x,gnu11,gnu1x,c17,iso9899:2017,c18,iso9899:2018,gnu17,gnu18,c2x,gnu2x,c++98,c++03,gnu++98,gnu++03,c++11,c++0x,gnu++11,gnu++0x,c++14,c++1y,gnu++14,gnu++1y,c++17,c++1z,gnu++17,gnu++1z,c++20,c++2a,gnu++20,gnu++2a,c++2b,gnu++2b,cl1.0,cl,cl1.1,cl1.2,cl2.0,cl3.0,clc++1.0,clc++,clc++2021,CL,CL1.1,CL1.2,CL2.0,CL3.0,CLC++,CLC++1.0,CLC++2021,cuda,hip,hlsl,hlsl2015,hlsl2016,hlsl2017,hlsl2018,hlsl2021,hlsl202x,}
+! HELP-NEXT:    Language standard to compile for
 ! HELP-NEXT: -S                     Only run preprocess and compilation steps
 ! HELP-NEXT: --target=<value>       Generate code for the given target
 ! HELP-NEXT: -U <macro>             Undefine macro <macro>
 ! HELP-NEXT: --version              Print version information
 ! HELP-NEXT: -W<warning>            Enable the specified warning
 ! HELP-NEXT: -Xflang <arg>          Pass <arg> to the flang compiler
 
 !-------------------------------------------------------------

 ! HELP-FC1-NEXT: -mllvm <value>         Additional arguments to forward to LLVM's option processing
 ! HELP-FC1-NEXT: -module-dir <dir>      Put MODULE files in <dir>
 ! HELP-FC1-NEXT: -module-suffix <suffix> Use <suffix> as the suffix for module files (the default value is `.mod`)
 ! HELP-FC1-NEXT: -nocpp                 Disable predefined and command line preprocessor macros
 ! HELP-FC1-NEXT: -o <file>              Write output to <file>
 ! HELP-FC1-NEXT: -pedantic              Warn on language extensions
 ! HELP-FC1-NEXT: -plugin <name>         Use the named plugin action instead of the default action (use "help" to list available options)
 ! HELP-FC1-NEXT: -P                     Disable linemarker output in -E mode
-! HELP-FC1-NEXT: -std=<value>           Language standard to compile for
+! HELP-FC1-NEXT: -std={c89,c90,iso9899:1990,iso9899:199409,gnu89,gnu90,c99,iso9899:1999,c9x,iso9899:199x,gnu99,gnu9x,c11,iso9899:2011,c1x,iso9899:201x,gnu11,gnu1x,c17,iso9899:2017,c18,iso9899:2018,gnu17,gnu18,c2x,gnu2x,c++98,c++03,gnu++98,gnu++03,c++11,c++0x,gnu++11,gnu++0x,c++14,c++1y,gnu++14,gnu++1y,c++17,c++1z,gnu++17,gnu++1z,c++20,c++2a,gnu++20,gnu++2a,c++2b,gnu++2b,cl1.0,cl,cl1.1,cl1.2,cl2.0,cl3.0,clc++1.0,clc++,clc++2021,CL,CL1.1,CL1.2,CL2.0,CL3.0,CLC++,CLC++1.0,CLC++2021,cuda,hip,hlsl,hlsl2015,hlsl2016,hlsl2017,hlsl2018,hlsl2021,hlsl202x,}
+! HELP-FC1-NEXT:    Language standard to compile for
 ! HELP-FC1-NEXT: -S                     Only run preprocess and compilation steps
 ! HELP-FC1-NEXT: -test-io               Run the InputOuputTest action. Use for development and testing only.
 ! HELP-FC1-NEXT: -triple <value>        Specify target triple (e.g. i686-apple-darwin9)
 ! HELP-FC1-NEXT: -U <macro>             Undefine macro <macro>
 ! HELP-FC1-NEXT: -version               Print the compiler version
 ! HELP-FC1-NEXT: -W<warning>            Enable the specified warning
 
 !---------------
 ! EXPECTED ERROR
 !---------------
 ! ERROR: error: unknown argument '-helps'; did you mean '-help'

llvm/include/llvm/Option/OptTable.h

public:
   }
 
   /// Get the meta-variable name to use when describing
   /// this options values in the help text.
   const char *getOptionMetaVar(OptSpecifier id) const {
     return getInfo(id).MetaVar;
   }
 
+  /// Get the values this option can take.
+  const char *getOptionValues(OptSpecifier id) const {
+    return getInfo(id).Values;
+  }
+
   /// Specify the environment variable where initial options should be read.
   void setInitialOptionsFromEnvironment(const char *E) { EnvVar = E; }
 
   /// Support grouped short options. e.g. -ab represents -a -b.
   void setGroupedShortOptions(bool Value) { GroupedShortOptions = Value; }
 
   /// Find possible value for given flags. This is used for shell
   /// autocompletion.

llvm/lib/Option/OptTable.cpp

static std::string getOptionHelpName(const OptTable &Opts, OptSpecifier Id) {
   case Option::SeparateClass: case Option::JoinedOrSeparateClass:
   case Option::RemainingArgsClass: case Option::RemainingArgsJoinedClass:
     Name += ' ';
     LLVM_FALLTHROUGH;
   case Option::JoinedClass: case Option::CommaJoinedClass:
   case Option::JoinedAndSeparateClass:
     if (const char *MetaVarName = Opts.getOptionMetaVar(Id))
       Name += MetaVarName;
-    else
+    else if (const char *Values = Opts.getOptionValues(Id)) {
+      Name += ("{" + Twine(Values) + "}").str();
+    } else
       Name += "<value>";
     break;
   }
 
   return Name;
 }
 
 namespace {
 struct OptionInfo {

llvm/unittests/Option/OptionParsingTest.cpp

TEST(Option, OptionParsing) {
   std::vector<std::string> Gs = AL.getAllArgValues(OPT_G);
   EXPECT_EQ("chuu", Gs[0]);
   EXPECT_EQ("2", Gs[1]);
 
   // Check the help text.
   std::string Help;
   raw_string_ostream RSO(Help);
   T.printHelp(RSO, "test", "title!");
-  EXPECT_NE(std::string::npos, Help.find("-A"));
+  std::string RefHelp = "OVERVIEW: title!\n"
+                        "\n"
+                        "USAGE: test\n"
+                        "\n"
+                        "OPTIONS:\n"
+                        "  -A                    The A option\n"
+                        "  --blarn               The blarn option\n"
+                        "  -blorp                The blorp option\n"
+                        "  -BB                   The B option\n"
+                        "  /cramb:CRAMB          The cramb option\n"
+                        "  -C C                  The C option\n"
+                        "  /C C                  The C option\n"
+                        "  -doopf1               The doopf1 option\n"
+                        "  -doopf2               The doopf2 option\n"
+                        "  -DD                   The D option\n"
+                        "  --ermghERMGH          The ermgh option\n"
+                        "  --fjormp              The fjormp option\n"
+                        "  -F F                  The F option\n"
+                        "  -GG                   The G option\n"
+                        "  --with-values={1,2,3} A valuable option\n";
+  EXPECT_EQ(Help, RefHelp);
 
   // Check usage line.
   T.printHelp(RSO, "name [options] file...", "title!");
   EXPECT_NE(std::string::npos, Help.find("USAGE: name [options] file...\n"));
 
   // Test aliases.
   auto Cs = AL.filtered(OPT_C);
   ASSERT_NE(Cs.begin(), Cs.end());

llvm/unittests/Option/Opts.td

 
 def Blorp : Flag<["-", "--"], "blorp">, HelpText<"The blorp option">, Flags<[OptFlag1]>;
 def Blarn : Flag<["--", "-"], "blarn">, HelpText<"The blarn option">, Flags<[OptFlag1]>;
 def Cramb : Joined<["/"], "cramb:">, HelpText<"The cramb option">, MetaVarName<"CRAMB">, Flags<[OptFlag1]>;
 def Doopf1 : Flag<["-"], "doopf1">, HelpText<"The doopf1 option">, Flags<[OptFlag1]>;
 def Doopf2 : Flag<["-"], "doopf2">, HelpText<"The doopf2 option">, Flags<[OptFlag2]>;
 def Ermgh : Joined<["--"], "ermgh">, HelpText<"The ermgh option">, MetaVarName<"ERMGH">, Flags<[OptFlag1]>;
 def Fjormp : Flag<["--"], "fjormp">, HelpText<"The fjormp option">, Flags<[OptFlag1]>;
+def WithValues : Joined<["--"], "with-values=">, HelpText<"A valuable option">, Flags<[OptFlag1]>, Values<"1,2,3">;
 
 def Glorrmp_eq : Flag<["--"], "glorrmp=">;
 
 def Blurmpq : Flag<["--"], "blurmp">;
 def Blurmpq_eq : Flag<["--"], "blurmp=">;
 
 def DashDash : Option<["--"], "", KIND_REMAINING_ARGS>;