From dc096a66cb519532121fb0fbedb13265bd4b29ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1vid=20Bolvansk=C3=BD?= Date: Sat, 8 Aug 2020 19:22:44 +0200 Subject: [PATCH] [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/Basic/DiagnosticSemaKinds.td | 7 ++ clang/lib/Sema/SemaExpr.cpp | 15 +++ clang/test/Sema/string-concat.c | 102 ++++++++++++++++++ 3 files changed, 124 insertions(+) create mode 100644 clang/test/Sema/string-concat.c diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 659f1d6df39e..7fe9396dbfc3 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/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; +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>; 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; diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 28da268e637f..e86c5b919698 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/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(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 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); + } } } diff --git a/clang/test/Sema/string-concat.c b/clang/test/Sema/string-concat.c new file mode 100644 index 000000000000..bf8369b079c8 --- /dev/null +++ b/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" +};