[cfe-commits] r157659 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp test/SemaCXX/warn-memset-bad-sizeof-fixit.cpp

[Anna Zaks]

On May 30, 2012, at 4:26 PM, David Blaikie wrote:

> 
> 
> On Wed, May 30, 2012 at 1:14 PM, Anna Zaks <ganna at apple.com> wrote:
> 
> On May 29, 2012, at 6:10 PM, Richard Smith wrote:
> 
>> Hi,
>> 
>> On Tue, May 29, 2012 at 5:34 PM, Anna Zaks <ganna at apple.com> wrote:
>> Author: zaks
>> Date: Tue May 29 19:34:21 2012
>> New Revision: 157659
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=157659&view=rev
>> Log:
>> Add fixits for memory access warnings.
>> 
>> We have a rule that fix-its on warnings aren't supposed to change the code's semantics (-Werror -fixit shouldn't change the semantics of a valid program). Consequently, fix-its such as this are usually placed on an accompanying note.
>>  
> 
> We discussed this fixit internally, and reached a consensus that we should not issue a fixit in this case.
> The warning only warns on the following code, where the user most likely forgot to both dereference AND multiply by the number of elements. 
> 
> It's likely they forgot the number of elements just because memcpy is usually used to copy several elements

This is it.

> - or is there some other reason to believe the suggested fixit isn't really adequate?
> 
> I'm curious why we would suggest what amounts to a fixit in the diagnostic, but not as an actual fixit in a note (indeed the sentence fragment in the diagnostic could be pulled out into the text of the note).

The proposal below is to extend the text proposal with the "(and multiply it by the number of elements)". It would be hard to issue a fixit in case they forgot to include the number of elements.

> 
> - David
>  
>  memcpy(to, from, sizeof(to));
> 
> Note, we currently do not (but should) warn on the following, where the fixit would be more appropriate.
>  memcpy(to, from, sizeof(to)*N);
> 
> I plan to make the warning message more user-friendly:
> -  warning: argument to 'sizeof' in 'memcpy' call is the same expression as the destination; did you mean to dereference it?
> -  memcpy(to, from, sizeof(to))
> -                   ~~                      ~~
> 
> +  warning: 'memcpy' call operates on objects of type 'Blah' while the size is based on a different type 'Blah*'. 
> +  memcpy(to, from, sizeof(to))
> +                   ~~                      ~~
> +  note: did you mean to dereference the argument to 'sizeof' (and multiply it by the number of elements)?
> 
> Anna.
>> Also, do not display the builtin name and macro expansion when the
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120530/e988742f/attachment.html>