----- 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
>
> Jordan
>
>
> On Jan 9, 2013, at 15:00 , Hal Finkel <[email protected]> wrote:
>
> > ----- Original Message -----
> >> From: "Eli Friedman" <[email protected]>
> >> To: "Hal Finkel" <[email protected]>
> >> Cc: "llvm cfe" <[email protected]>
> >> 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 <[email protected]>
> >> 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
> > <first-line-indent-v2.patch>_______________________________________________
> > cfe-commits mailing list
> > [email protected]
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
diff --git a/lib/Frontend/PrintPreprocessedOutput.cpp b/lib/Frontend/PrintPreprocessedOutput.cpp
index cc8d935..e7b6497 100644
--- a/lib/Frontend/PrintPreprocessedOutput.cpp
+++ b/lib/Frontend/PrintPreprocessedOutput.cpp
@@ -139,11 +139,15 @@ public:
diag::Mapping Map, StringRef Str);
bool HandleFirstTokOnLine(Token &Tok);
+
+ /// Move to the line of the provided source location. This will
+ /// return true if the output stream required adjustment or if
+ /// the requested location is on the first line.
bool MoveToLine(SourceLocation Loc) {
PresumedLoc PLoc = SM.getPresumedLoc(Loc);
if (PLoc.isInvalid())
return false;
- return MoveToLine(PLoc.getLine());
+ return MoveToLine(PLoc.getLine()) || (PLoc.getLine() == 1);
}
bool MoveToLine(unsigned LineNo);
diff --git a/test/Preprocessor/first-line-indent.c b/test/Preprocessor/first-line-indent.c
new file mode 100644
index 0000000..d220d57
--- /dev/null
+++ b/test/Preprocessor/first-line-indent.c
@@ -0,0 +1,7 @@
+ foo
+// RUN: %clang_cc1 -E %s | FileCheck -strict-whitespace %s
+ bar
+
+// CHECK: {{^ }}foo
+// CHECK: {{^ }}bar
+
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
