[clang][cli] Port a CommaJoined option to the marshalling infrastructure

[dexonsmith]

LGTM, with one nit.

clang/lib/Frontend/CompilerInvocation.cpp

333–337

The comparison to .front is a bit subtle. I think this might be more direct / easy to read using drop_begin:

CommaJoinedValue.append(Values.front());
for (const std::string &Value : llvm::drop_begin(Values, 1)) {
  CommaJoinedValue.append(",");
  CommaJoinedValue.append(Value);
}

clang/include/clang/Driver/Options.td

 def fdiagnostics_color : Flag<["-"], "fdiagnostics-color">, Group<f_Group>,
   Flags<[CoreOption, NoXarchOption]>;
 def fdiagnostics_color_EQ : Joined<["-"], "fdiagnostics-color=">, Group<f_Group>;
 def fansi_escape_codes : Flag<["-"], "fansi-escape-codes">, Group<f_Group>,
   Flags<[CoreOption, CC1Option]>, HelpText<"Use ANSI escape codes for diagnostics">,
   MarshallingInfoFlag<"DiagnosticOpts->UseANSIEscapeCodes">;
 def fcomment_block_commands : CommaJoined<["-"], "fcomment-block-commands=">, Group<f_clang_Group>, Flags<[CC1Option]>,
   HelpText<"Treat each comma separated argument in <arg> as a documentation comment block command">,
-  MetaVarName<"<arg>">;
+  MetaVarName<"<arg>">, MarshallingInfoStringVector<"LangOpts->CommentOpts.BlockCommandNames">;
 def fparse_all_comments : Flag<["-"], "fparse-all-comments">, Group<f_clang_Group>, Flags<[CC1Option]>,
   MarshallingInfoFlag<"LangOpts->CommentOpts.ParseAllComments">;
 def frecord_command_line : Flag<["-"], "frecord-command-line">,
   Group<f_clang_Group>;
 def fno_record_command_line : Flag<["-"], "fno-record-command-line">,
   Group<f_clang_Group>;
 def : Flag<["-"], "frecord-gcc-switches">, Alias<frecord_command_line>;
 def : Flag<["-"], "fno-record-gcc-switches">, Alias<fno_record_command_line>;

clang/lib/Frontend/CompilerInvocation.cpp

 }
 
 static void denormalizeStringVector(SmallVectorImpl<const char *> &Args,
                                     const char *Spelling,
                                     CompilerInvocation::StringAllocator SA,
                                     Option::OptionClass OptClass,
                                     unsigned TableIndex,
                                     const std::vector<std::string> &Values) {
-  for (const std::string &Value : Values) {
+  if (OptClass == Option::OptionClass::CommaJoinedClass) {
+    std::string CommaJoinedValue;
+    if (!Values.empty()) {
+      CommaJoinedValue.append(Values.front());
+      for (const std::string &Value : llvm::drop_begin(Values, 1)) {
+        CommaJoinedValue.append(",");
+        CommaJoinedValue.append(Value);
+      }
+    }
+    denormalizeString(Args, Spelling, SA, Option::OptionClass::JoinedClass,
+                      TableIndex, CommaJoinedValue);
+  } else if (OptClass == Option::OptionClass::JoinedClass) {
+    for (const std::string &Value : Values)
       denormalizeString(Args, Spelling, SA, OptClass, TableIndex, Value);
   }
 }
 
 static Optional<std::string> normalizeTriple(OptSpecifier Opt, int TableIndex,
                                              const ArgList &Args,
                                              DiagnosticsEngine &Diags) {
   auto *Arg = Args.getLastArg(Opt);
   if (!Arg)
#undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE
 
   if (!AnOpts.ModelPath.empty() &&
       !llvm::sys::fs::is_directory(AnOpts.ModelPath))
     Diags->Report(diag::err_analyzer_config_invalid_input) << "model-path"
                                                            << "a filename";
 }
 
 static void ParseCommentArgs(CommentOptions &Opts, ArgList &Args) {
-  Opts.BlockCommandNames = Args.getAllArgValues(OPT_fcomment_block_commands);
   Opts.ParseAllComments = Args.hasArg(OPT_fparse_all_comments);
 }
 
 /// Create a new Regex instance out of the string value in \p RpassArg.
 /// It returns a pointer to the newly generated Regex instance.
 static std::shared_ptr<llvm::Regex>
 GenerateOptimizationRemarkRegex(DiagnosticsEngine &Diags, ArgList &Args,
                                 Arg *RpassArg) {

clang/unittests/Frontend/CompilerInvocationTest.cpp

TEST_F(CommandLineTest, StringVectorMultiple) {
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
 
   ASSERT_THAT(GeneratedArgs, ContainsN(StrEq("-fmodule-map-file=a"), 1));
   ASSERT_THAT(GeneratedArgs, ContainsN(StrEq("-fmodule-map-file=b"), 1));
   ASSERT_THAT(GeneratedArgs, ContainsN(HasSubstr("-fmodule-map-file"), 2));
 }
 
+// CommaJoined option with MarshallingInfoStringVector.
+
+TEST_F(CommandLineTest, StringVectorCommaJoinedNone) {
+  const char *Args[] = {""};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_TRUE(Invocation.getLangOpts()->CommentOpts.BlockCommandNames.empty());
+
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+
+  ASSERT_THAT(GeneratedArgs,
+              Not(Contains(HasSubstr("-fcomment-block-commands"))));
+}
+
+TEST_F(CommandLineTest, StringVectorCommaJoinedSingle) {
+  const char *Args[] = {"-fcomment-block-commands=x,y"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_EQ(Invocation.getLangOpts()->CommentOpts.BlockCommandNames,
+            std::vector<std::string>({"x", "y"}));
+
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+
+  ASSERT_THAT(GeneratedArgs,
+              ContainsN(StrEq("-fcomment-block-commands=x,y"), 1));
+}
+
+TEST_F(CommandLineTest, StringVectorCommaJoinedMultiple) {
+  const char *Args[] = {"-fcomment-block-commands=x,y",
+                        "-fcomment-block-commands=a,b"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_EQ(Invocation.getLangOpts()->CommentOpts.BlockCommandNames,
+            std::vector<std::string>({"x", "y", "a", "b"}));
+
+  Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+
+  ASSERT_THAT(GeneratedArgs,
+              ContainsN(StrEq("-fcomment-block-commands=x,y,a,b"), 1));
+}
+
 // A flag that should be parsed only if a condition is met.
 
 TEST_F(CommandLineTest, ConditionalParsingIfFalseFlagNotPresent) {
   const char *Args[] = {""};
 
   CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
 
   ASSERT_FALSE(Diags->hasErrorOccurred());