[cfe-commits] r86962 - /cfe/trunk/tools/clang-cc/clang-cc.cpp

[Douglas Gregor]

On Nov 12, 2009, at 5:38 PM, Chris Lattner wrote:

> On Nov 12, 2009, at 4:29 PM, Douglas Gregor wrote:
>>>> So perhaps we should turn -fixit into an action instead of  
>>>> relying on
>>>> users not to do anything stupid with it.
>>>
>>> I think that this is a serious problem with a different fix.  Fixits
>>> for warnings should be to 'make the warning go away without changing
>>> the semantics of the code' not 'change my code to make the warning  
>>> go
>>> away'.
>>
>> Agreed.
>
> Another way of putting it is that fixits should always agree with  
> the way the compiler recovers.  In the case of a warning, this  
> should be how it actually parsed the code.

Precisely.

>>
>>> In this case, the fixit should insert:
>>> i & (1 == 0)
>>>
>>> No?  Maybe there should be two modes for fixits: semantic preserving
>>> ones that can always be safely applied vs potential bugs?
>>
>>
>> I like the idea of tagging those fix-its that actually change  
>> behavior. They should not be automatically applied by the -fixit  
>> mode.
>>
>> We could even provide multiple alternatives via different nodes:
>>
>>
>> 	note: to silence this warning, insert paretheses around the ==  
>> expression
>>
>> (show 1 == 0 underlined with the fixit adding ( and ) around it.  
>> this fixit doesn't change behavior)
>>
>> 	note: to evaluate the & first, insert parentheses around the &  
>> expression
>>
>> (show i & 1 underlined with the fixit adding ( and ) around it.  
>> this fixit does change behavior)
>
> I think this is a great idea.  Both "answers" should be attached to  
> the same diagnostic, then a UI could provide "two buttons" or  
> something.

Implemented here via notes:

	http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20100104/026110.html

	- Doug