[cfe-commits] r67823 - /cfe/trunk/lib/Sema/SemaType.cpp

[Chris Lattner]

On Mar 27, 2009, at 8:34 AM, Douglas Gregor wrote:
>> Ok, well that's kind of the design of the fixit advice:  
>> theoretically and IDE would only show fixit advice if all generated  
>> errors had fix it's.  If any errors were produced without fixit  
>> advice, then we'd assume that error recovery got confused and that  
>> none of the fixit advice is trustworthy.
>
> I'm about 85% convinced about that. For me, it depends on just how  
> accurate our fix-it advice turns out to be, and I'm still going to  
> argue that if we can't be reasonably accurate, we shouldn't suggest  
> anything.

Sure, I completely agree.  This is why we have to be really careful  
that doing what the fixit[s] says will actually cause zero errors from  
the next build.

Incidentally, this patch:
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20090323/014595.html

has another problem that I didn't comment on.  I don't think that  
errors with fixit advice should *ever* return ExprError.  For example,  
if you have:

X->Y = 4;

and X is a function returning a pointer to a struct with an 'int Y',  
then adding () will fix the problem.  However, if it is a pointer to a  
struct which 'struct a Y', then adding () won't fix it.

In this case, I think that Sema should output the fixit advice, and  
then synthesize the right AST nodes that would be built if the user  
did add ()'s.  In the later case, this means that we'll get a second  
error, and that error won't have fix-it advice.  Therefore that  
invalidates the previous fixit advice.

Does this seem reasonable?

>> That would be nice refinement in any case, I guess, but the meta  
>> issue stands: fixit advice is only trustworthy when the front-end  
>> isn't hopelessly confused.
>
> I agree. Perhaps it's just a question of whether we suppress the fix- 
> it advice from the front end itself, or whether we rely on the  
> diagnostic engine to do it. In an IDE, we're assuming that the  
> suppression will happen after the front-end has completed its  
> execution.

Right.  I think an IDE should do this.  The DiagClient can choose  
whether or not to display it.

-Chris