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

[Chris Lattner]

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.

>
>> 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.

-Chris