[cfe-commits] [PATCH] Correct first-line indentation in preprocessor output

[Hal Finkel]

Ping.

 -Hal

----- Original Message -----
> From: "Hal Finkel" <hfinkel at anl.gov>
> To: "Eli Friedman" <eli.friedman at gmail.com>
> Cc: "llvm cfe" <cfe-commits at cs.uiuc.edu>
> Sent: Wednesday, January 16, 2013 11:45:56 AM
> Subject: Re: [cfe-commits] [PATCH] Correct first-line indentation in preprocessor output
> 
> Ping.
> 
> Thanks again,
> Hal
> 
> ----- Original Message -----
> > From: "Hal Finkel" <hfinkel at anl.gov>
> > To: "Eli Friedman" <eli.friedman at gmail.com>
> > Cc: "llvm cfe" <cfe-commits at cs.uiuc.edu>
> > Sent: Wednesday, January 9, 2013 5:00:14 PM
> > Subject: Re: [cfe-commits] [PATCH] Correct first-line indentation
> > in preprocessor output
> > 
> > ----- Original Message -----
> > > From: "Eli Friedman" <eli.friedman at gmail.com>
> > > To: "Hal Finkel" <hfinkel at anl.gov>
> > > Cc: "llvm cfe" <cfe-commits at cs.uiuc.edu>
> > > Sent: Wednesday, January 9, 2013 4:40:18 PM
> > > Subject: Re: [cfe-commits] [PATCH] Correct first-line indentation
> > > in preprocessor output
> > > 
> > > On Wed, Jan 9, 2013 at 2:20 PM, Hal Finkel <hfinkel at anl.gov>
> > > wrote:
> > > > Currently, the -E output from clang does not produce the
> > > > correct
> > > > indentation on the first line. This is because MoveToLine
> > > > returns
> > > > false, and when this happens, the regular process for producing
> > > > initial indentation is skipped. This patch makes sure this does
> > > > not happen on the first line -- it is not clear to me whether
> > > > there are other circumstances where the current logic could be
> > > > problematic.
> > > >
> > > > It looks like calling SourceManager::getPresumedLoc is a
> > > > relatively
> > > > expensive operation, so however this is fixed, I assume that we
> > > > want to minimize calls to that function.
> > > >
> > > > Please review.
> > > 
> > > --- a/lib/Frontend/PrintPreprocessedOutput.cpp
> > > +++ b/lib/Frontend/PrintPreprocessedOutput.cpp
> > > @@ -139,10 +139,13 @@ public:
> > >                                  diag::Mapping Map, StringRef
> > >                                  Str);
> > > 
> > >    bool HandleFirstTokOnLine(Token &Tok);
> > > -  bool MoveToLine(SourceLocation Loc) {
> > > +  bool MoveToLine(SourceLocation Loc, bool *FirstLine = 0) {
> > >      PresumedLoc PLoc = SM.getPresumedLoc(Loc);
> > > -    if (PLoc.isInvalid())
> > > +    if (PLoc.isInvalid()) {
> > > +      if (FirstLine) *FirstLine = false;
> > >        return false;
> > > +    }
> > > +    if (FirstLine) *FirstLine = PLoc.getLine() == 1;
> > >      return MoveToLine(PLoc.getLine());
> > >    }
> > >    bool MoveToLine(unsigned LineNo);
> > > 
> > > There is precisely one user of the return value of MoveToLine:
> > > the
> > > one
> > > you're modifying.  Why do we need an extra out parameter?
> > 
> > Good point, thanks! Revised patch attached -- which now changes one
> > source line ;)
> > 
> >  -Hal
> > 
> > > 
> > > -Eli
> > > 
> > 
> > --
> > Hal Finkel
> > Postdoctoral Appointee
> > Leadership Computing Facility
> > Argonne National Laboratory
> > 
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> > 
> 
> --
> Hal Finkel
> Postdoctoral Appointee
> Leadership Computing Facility
> Argonne National Laboratory
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: first-line-indent-v2.patch
Type: text/x-patch
Size: 889 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130124/e268e3e0/attachment.bin>