[Diagnostics] Diagnose missing comma in string array initialization

Motivation (from PR37674):

const char *ss[] = {
  "foo", "bar",
  "baz", "qux"  // <-- Missing comma!
  "abc", "xyz"
  };

This kind of bug was recently also found in LLVM codebase (see PR47030).

Solves PR47038, PR37674

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D85545

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -3002,6 +3002,10 @@ def err_missing_atsign_prefix : Error<
 def warn_objc_string_literal_comparison : Warning<
   "direct comparison of a string literal has undefined behavior">,
   InGroup<ObjCStringComparison>;
+def warn_concatenated_literal_array_init : Warning<
+  "suspicious concatenation of string literals in an array initialization; "
+  "did you mean to separate the elements with a comma?">,
+  InGroup<DiagGroup<"string-concatenation">>;
 def warn_concatenated_nsarray_literal : Warning<
   "concatenated NSString literal for an NSArray expression - "
   "possibly missing a comma">,
@@ -6142,6 +6146,9 @@ def warn_overloaded_shift_in_comparison :Warning<
 def note_evaluate_comparison_first :Note<
   "place parentheses around comparison expression to evaluate it first">;
 
+def note_concatenated_string_literal_silence :Note<
+  "place parentheses around the string literal to silence warning">;
+
 def warn_addition_in_bitshift : Warning<
   "operator '%0' has lower precedence than '%1'; "
   "'%1' will be evaluated first">, InGroup<ShiftOpParentheses>;

clang/lib/Sema/SemaExpr.cpp

@@ -6906,6 +6906,21 @@ Sema::ActOnInitList(SourceLocation LBraceLoc, MultiExprArg InitArgList,
         << DIE->getSourceRange();
       Diag(InitArgList[I]->getBeginLoc(), diag::note_designated_init_mixed)
         << InitArgList[I]->getSourceRange();
+    } else if (const auto *SL = dyn_cast<StringLiteral>(InitArgList[I])) {
+      unsigned NumConcat = SL->getNumConcatenated();
+      // Diagnose missing comma in string array initialization.
+      // Do not warn when all the elements in the initializer are concatenated together.
+      // Do not warn for macros too.
+      if (NumConcat > 1 && E > 1 && !SL->getBeginLoc().isMacroID()) {
+        SmallVector<FixItHint, 1> Hints;
+        for (unsigned i = 0; i < NumConcat - 1; ++i)
+          Hints.push_back(FixItHint::CreateInsertion(
+              PP.getLocForEndOfToken(SL->getStrTokenLoc(i)), ", "));
+
+        Diag(SL->getStrTokenLoc(1), diag::warn_concatenated_literal_array_init)
+            << Hints;
+        Diag(SL->getBeginLoc(), diag::note_concatenated_string_literal_silence);
+      }
     }
   }
 

clang/test/Sema/string-concat.c

@@ -0,0 +1,102 @@
+
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s
+
+const char *missing_comma[] = {
+    "basic_filebuf",
+    "basic_ios",
+    "future",
+    "optional",
+    "packaged_task" // expected-note{{place parentheses around the string literal to silence warning}}
+    "promise",      // expected-warning{{suspicious concatenation of string literals in an array initialization; did you mean to separate the elements with a comma?}}     
+    "shared_future"
+};
+
+#ifndef __cplusplus
+typedef __WCHAR_TYPE__ wchar_t;
+#endif
+
+const wchar_t *missing_comma_wchar[] = {
+    L"basic_filebuf",
+    L"packaged_task" // expected-note{{place parentheses around the string literal to silence warning}}
+    L"promise"      // expected-warning{{suspicious concatenation of string literals in an array initialization; did you mean to separate the elements with a comma?}}
+};
+
+const char *missing_comma_u8[] = {
+    u8"basic_filebuf",
+    u8"packaged_task" // expected-note{{place parentheses around the string literal to silence warning}}
+    u8"promise"      // expected-warning{{suspicious concatenation of string literals in an array initialization; did you mean to separate the elements with a comma?}}
+};
+
+const char *missing_two_commas[] = {"basic_filebuf",
+                       "basic_ios" // expected-note{{place parentheses around the string literal to silence warning}}
+                       "future"    // expected-warning{{suspicious concatenation of string literals in an array initialization; did you mean to separate the elements with a comma?}}
+                       "optional",
+                       "packaged_task"};
+
+const char *missing_comma_same_line[] = {"basic_filebuf", "basic_ios",
+                       "future" "optional",         // expected-note{{place parentheses around the string literal to silence warning}}
+                       "packaged_task", "promise"}; // expected-warning@-1{{suspicious concatenation of string literals in an array initialization; did you mean to separate the elements with a comma?}}
+
+const char *missing_comma_different_lines[] = {"basic_filebuf", "basic_ios" // expected-note{{place parentheses around the string literal to silence warning}}
+                       "future", "optional",        // expected-warning{{suspicious concatenation of string literals in an array initialization; did you mean to separate the elements with a comma?}}
+                       "packaged_task", "promise"};
+
+const char *missing_comma_same_line_all_literals[] = {"basic_filebuf", "future" "optional", "packaged_task"}; // expected-note{{place parentheses around the string literal to silence warning}}
+                                                                               // expected-warning@-1{{suspicious concatenation of string literals in an array initialization; did you mean to separate the elements with a comma?}}
+
+char missing_comma_inner[][4] = {
+    "a",
+    "b" // expected-note{{place parentheses around the string literal to silence warning}}
+    "c" // expected-warning{{suspicious concatenation of string literals in an array initialization; did you mean to separate the elements with a comma?}}
+};
+
+
+#define ONE(x) x
+#define TWO "foo"
+const char *macro_test[] = { ONE("foo") "bar", 
+                             TWO "bar", 
+                             "foo" TWO // expected-note{{place parentheses around the string literal to silence warning}}
+                           };          // expected-warning@-1{{suspicious concatenation of string literals in an array initialization; did you mean to separate the elements with a comma?}}
+
+// Do not warn for macros.
+
+#define BASIC_IOS "basic_ios"
+#define FUTURE "future"
+const char *macro_test2[] = {"basic_filebuf", BASIC_IOS
+                        FUTURE, "optional",
+                       "packaged_task", "promise"};
+
+#define FOO(xx) xx "_normal", \
+                xx "_movable",
+
+const char *macro_test3[] = {"basic_filebuf",
+                       "basic_ios",
+                       FOO("future")
+                       "optional",
+                       "packaged_task"};
+
+#define BAR(name) #name "_normal"
+
+const char *macro_test4[] = {"basic_filebuf",
+                       "basic_ios",
+                       BAR(future),
+                       "optional",
+                       "packaged_task"};
+
+#define SUPPRESS(x) x
+const char *macro_test5[] = { SUPPRESS("foo" "bar"), "baz" };
+
+// Do not warn when all the elements in the initializer are concatenated together.
+const char *all_elems_in_init_concatenated[] = {"a" "b" "c"};
+
+// Warning can be supressed also by extra parentheses.
+const char *extra_parens_to_suppress_warning[] = {
+    "basic_filebuf",
+    "basic_ios",
+    "future",
+    "optional",
+    ("packaged_task"
+    "promise"),
+    "shared_future"
+};