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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
