AaronBallman wrote:

> For what it's worth, this causes `-Wstring-concatenation` to pop up a few 
> times in the Linux kernel. Two of the instances are simple enough to resolve 
> under the kernel's "Do not split user visible strings regardless of length" 
> rule.
> 
> ```
> drivers/scsi/lpfc/lpfc_attr.c:76:3: warning: suspicious concatenation of 
> string literals in an array initialization; did you mean to separate the 
> elements with a comma? [-Wstring-concatenation]
>    75 |         "link negotiated speed does not match existing"
>       |
>       |                                                        ,
>    76 |                 " trunk - link was \"high\" speed",
>       |                 ^
> drivers/scsi/lpfc/lpfc_attr.c:75:2: note: place parentheses around the string 
> literal to silence warning
>    75 |         "link negotiated speed does not match existing"
>       |         ^
> ```
> 
> ```c
> const char *const trunk_errmsg[] = {    /* map errcode */
>     "", /* There is no such error code at index 0*/
>     "link negotiated speed does not match existing"
>         " trunk - link was \"low\" speed",
>     "link negotiated speed does not match"
>         " existing trunk - link was \"middle\" speed",
>     "link negotiated speed does not match existing"
>         " trunk - link was \"high\" speed",
>     "Attached to non-trunking port - F_Port",
>     "Attached to non-trunking port - N_Port",
>     "FLOGI response timeout",
>     "non-FLOGI frame received",
>     "Invalid FLOGI response",
>     "Trunking initialization protocol",
>     "Trunk peer device mismatch",
> };
> ```

That looks like perfectly reasonable code we probably should not be warning on 
IMO. Requiring the user to parenthesize it or concatenate it is kind of 
annoying. I wonder if another suppression mechanism would be to silence in the 
presence of a `\` line splice at the end of the line?

> However, this third one is a little interesting because the warning happens 
> on the last instance of a sequence of multi-line concatenations:
> 
> Maybe this warning could take into account using designators probably means 
> the concatenation is intentional? Not sure if that is possible.

I'm not certain that a designator is enough of a signal to mean the 
concatenation is intentional. The tradeoff here is false positives vs false 
negatives, and I think this is starting to show that we're introducing more 
false positives. Another idea would be to revert these changes and instead make 
a clang-tidy check that's more aggressive.

https://github.com/llvm/llvm-project/pull/154018
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to