----- Original Message -----
> From: "Jordan Rose" <jordan_r...@apple.com>
> To: "Hal Finkel" <hfin...@anl.gov>
> Cc: "Eli Friedman" <eli.fried...@gmail.com>, "llvm cfe"
> <cfe-commits@cs.uiuc.edu>
> Sent: Thursday, January 24, 2013 7:48:19 PM
> Subject: Re: [cfe-commits] [PATCH] Correct first-line indentation in
> preprocessor output
>
>
> On Jan 24, 2013, at 17:07 , Hal Finkel <hfin...@anl.gov> wrote:
>
> > ----- Original Message -----
> >> From: "Jordan Rose" <jordan_r...@apple.com>
> >> To: "Hal Finkel" <hfin...@anl.gov>
> >> Cc: "Eli Friedman" <eli.fried...@gmail.com>, "llvm cfe"
> >> <cfe-commits@cs.uiuc.edu>
> >> 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.
r173657. Thanks!
-Hal
>
> Jordan
>
>
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
