[clang] 975467e - [Diagnostics] Handle string concat pattern and avoid false positives

[Dávid Bolvanský via cfe-commits]

We warn if it is just one occurance and next literal is not split too. I changed test to test other scenario. I could add that test back, but I think we have enough tests to cover code.

We should not warn for code pattern:
{
“a” “b”,
“c” “d”
}




> Dňa 9. 8. 2020 o 17:49 užívateľ Arthur O'Dwyer <arthur.j.odwyer at gmail.com> napísal:
> 
> 
> Hi Dávid,
> Does this part of the patch imply that we don't want to warn on
>     "foo" TWO
> ? and if so, why not?
> Anyway, I think at least
>     "foo" TWO
> should be kept in the test suite, as a test-for-absence-of-warning.
> 
>                               TWO "bar", 
> -                             "foo" TWO // expected-note{{place parentheses around the string literal to silence warning}}
> +                             "foo" "bar" TWO // expected-note{{place parentheses around the string literal to silence warning}}
> 
> my $.02,
> Arthur
> 
>> On Sun, Aug 9, 2020 at 10:03 AM Dávid Bolvanský via cfe-commits <cfe-commits at lists.llvm.org> wrote:
> 
>> 
>> Author: Dávid Bolvanský
>> Date: 2020-08-09T16:02:41+02:00
>> New Revision: 975467e4aa7ce1b8fcf4af0e25cdf053cfa8669e
>> 
>> URL: https://github.com/llvm/llvm-project/commit/975467e4aa7ce1b8fcf4af0e25cdf053cfa8669e
>> DIFF: https://github.com/llvm/llvm-project/commit/975467e4aa7ce1b8fcf4af0e25cdf053cfa8669e.diff
>> 
>> LOG: [Diagnostics] Handle string concat pattern and avoid false positives
>> 
>> Added: 
>> 
>> 
>> Modified: 
>>     clang/lib/Sema/SemaExpr.cpp
>>     clang/test/Sema/string-concat.c
>> 
>> Removed: 
>> 
>> 
>> 
>> ################################################################################
>> diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
>> index 35047a7b2b14..74427f8cd7ae 100644
>> --- a/clang/lib/Sema/SemaExpr.cpp
>> +++ b/clang/lib/Sema/SemaExpr.cpp
>> @@ -6908,10 +6908,13 @@ Sema::ActOnInitList(SourceLocation LBraceLoc, MultiExprArg InitArgList,
>>          << InitArgList[I]->getSourceRange();
>>      } else if (const auto *SL = dyn_cast<StringLiteral>(InitArgList[I])) {
>>        unsigned NumConcat = SL->getNumConcatenated();
>> +      const auto *SLNext =
>> +          dyn_cast<StringLiteral>(InitArgList[I + 1 < E ? I + 1 : 0]);
>>        // 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 > 2 && !SL->getBeginLoc().isMacroID()) {
>> +      // Do not warn when all the elements in the initializer are concatenated
>> +      // together. Do not warn for macros too.
>> +      if (NumConcat > 1 && E > 2 && !SL->getBeginLoc().isMacroID() && SLNext &&
>> +          NumConcat != SLNext->getNumConcatenated()) {
>>          SmallVector<FixItHint, 1> Hints;
>>          for (unsigned i = 0; i < NumConcat - 1; ++i)
>>            Hints.push_back(FixItHint::CreateInsertion(
>> 
>> diff  --git a/clang/test/Sema/string-concat.c b/clang/test/Sema/string-concat.c
>> index c93bbd4eaa00..13e9656d2536 100644
>> --- a/clang/test/Sema/string-concat.c
>> +++ b/clang/test/Sema/string-concat.c
>> @@ -61,7 +61,7 @@ char missing_comma_inner[][5] = {
>>  #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}}
>> +                             "foo" "bar" TWO // expected-note{{place parentheses around the string literal to silence warning}}
>>                             };          // expected-warning at -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.
>> @@ -104,6 +104,12 @@ const char *not_warn[] = {
>>      "world", "test"
>>  };
>> 
>> +const char *not_warn2[] = {
>> +    "// Aaa\\\n"   " Bbb\\ \n"   " Ccc?" "?/\n",
>> +    "// Aaa\\\r\n" " Bbb\\ \r\n" " Ccc?" "?/\r\n",
>> +    "// Aaa\\\r"   " Bbb\\ \r"   " Ccc?" "?/\r"
>> +  };
>> +
>>  // Do not warn when all the elements in the initializer are concatenated together.
>>  const char *all_elems_in_init_concatenated[] = {"a" "b" "c"};
>> 
>> 
>> 
>> 
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200809/89484dd8/attachment.html>