[PATCH] [StaticAnalyzer] New checker StringPlusChar

[Jordan Rose]

Looking pretty good, though Richard should probably give final approval now. Is it worth checking that the string is actually a pointer to some kind of character, though? It's unlikely that you'd add a character to an int*, but the message is a bit off, then.

Also, the type in the message should be the type of the character literal (i.e. L'a', u'a', and U'a'), not a hardcoded "char". Unfortunately, in C that type is 'int', so in the C case you should check and see if the value fits in a char, and substitute CharTy in that case.

Jordan


On Oct 4, 2013, at 5:28 , Anders Rönnholm <Anders.Ronnholm at evidente.se> wrote:

> Hi,
> 
> A new iteration on the checker. I have moved it to be a warning in semaexpr now.
> 
> //Anders
> 
> .......................................................................................................................
> Anders Rönnholm Senior Engineer
> Evidente ES East AB  Warfvinges väg 34  SE-112 51 Stockholm  Sweden
> 
> Mobile:                    +46 (0)70 912 42 54
> E-mail:                    Anders.Ronnholm at evidente.se
> 
> www.evidente.se
> 
> ________________________________________
> Från: Jordan Rose [jordan_rose at apple.com]
> Skickat: den 2 oktober 2013 18:12
> Till: Daniel Marjamäki
> Cc: Richard Smith; Anders Rönnholm; cfe commits
> Ämne: Re: [PATCH] [StaticAnalyzer] New checker StringPlusChar
> 
> On Oct 1, 2013, at 22:16 , Daniel Marjamäki <Daniel.Marjamaki at evidente.se> wrote:
> 
>> 
>> Hello!
>> 
>>> This should be a warning, not a static analyser check, shouldn't it?
>> 
>> Is anybody against this?
>> 
>> Personally I feel very confident about this check regarding signal/noise. However putting it in the static analyser to start with felt like a safe choice.
> 
> I'm fine with this. I didn't realize at first it was just for character literals; with that restriction this does seem like false positives would be unlikely. Thanks, Richard. (Sorry to force another iteration, Anders. The existing warning is in SemaExpr.cpp: diagnoseStringPlusInt.)
> 
> Daniel, delaying += and char plus string seem fine to me, though they might just be handled if you hook into diagnoseStringPlusInt. And thanks for the reminder about things like s8, u8, etc.
> 
> Jordan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: stringpluschar.diff
Type: text/x-patch
Size: 5677 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131004/3fe8c71b/attachment.bin>
-------------- next part --------------