================
Comment at: lib/Format/TokenAnnotator.cpp:573
@@ -571,1 +572,3 @@
void determineTokenType(FormatToken &Current) {
+ if (Current.Previous && Current.Previous->Tok.is(tok::unknown))
+ Current.Type = TT_ImplicitStringLiteral;
----------------
Alexander Kornienko wrote:
> Daniel Jasper wrote:
> > Can this actually make a difference? If yes, how? It seems like we convert
> > tok::unknown to tok::string_literal or set Type = TT_ImplicitStringLiteral
> > earlier or merge this into another token's whitespace..
> I think, we want to preserve whitespace around non-whitespace unknown tokens.
> The most straightforward way to do this seems to be setting the type to
> TT_ImplicitStringLiteral for the token itself and the next one. The former is
> done in Format.cpp, and the latter - here.
>
> And, btw, we don't convert tok::unknown to tok::string_literal in this
> specific case. I'm not sure if we should.
Why would we want to preserve that whitespace? I'd just add the whitespace as
leading whitespace to the TT_ImplicitStringLiteral.
================
Comment at: lib/Format/TokenAnnotator.cpp:575
@@ +574,3 @@
+ Current.Type = TT_ImplicitStringLiteral;
+ if (Current.Type != TT_Unknown)
+ return;
----------------
Alexander Kornienko wrote:
> Daniel Jasper wrote:
> > This change is not a no-op!! Move to line 608 and add:
> >
> > // FIXME: Add tests that break if this gets moved up.
> >
> > if no tests are failing now...
> It wasn't meant to be a no-op. This check is here to preserve Current.Type if
> it is already set to TT_ImplicitStringLiteral. If moving the check here can
> really break something, we certainly need tests for this. I've added the
> FIXME line and changed the check to a more specific one.
I think you are actually not achieving what you want to achieve. The code
between here and line 614 does not even touch Current's Type. It only changes
the type of previous tokens (which might still be bad). However, this check in
its current form should not change the behavior. If it does, what is the test
case?
http://llvm-reviews.chandlerc.com/D1858
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
