On Jan 24, 2013, at 17:07 , Hal Finkel <[email protected]> wrote:

> ----- Original Message -----
>> From: "Jordan Rose" <[email protected]>
>> To: "Hal Finkel" <[email protected]>
>> Cc: "Eli Friedman" <[email protected]>, "llvm cfe" 
>> <[email protected]>
>> Sent: Thursday, January 24, 2013 11:55:20 AM
>> Subject: Re: [cfe-commits] [PATCH] Correct first-line indentation in 
>> preprocessor output
>> 
>> Hm. If this is the change we go with, there should definitiely be a
>> comment saying that MoveToLine(SourceLocation) returns a different
>> value than MoveToLine(unsigned). That, or they should be changed to
>> be consistent.
> 
> Good point.
> 
>> 
>> (My first inclination was to change
>> PrintPPOutputPPCallbacks::HandleFirstTokOnLine, and keep the current
>> semantics of MoveToLine, but then you'd have to call getPresumedLoc
>> again. Or not use the convenience wrapper for MoveToLine. But it's
>> probably okay since, as Eli pointed out, this is the only consumer
>> of the return value.)
> 
> Exactly.
> 
>> 
>> Also, you should probably add a ^ to your {{       }} in your test
>> case, just to make things stricter/easier for FileCheck.
> 
> Done.
> 
> Revised patch attached. How's this?
> 
> Thanks again,
> Hal

It's not exactly an area of the compiler I own, but it seems harmless enough 
and does fix an issue, so go ahead. If someone else has a stronger opinion on 
how to deal with MoveToLine it can be changed after the fact.

Jordan

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to