[PATCH] Don't remove unknown tokens that are not whitespace (PR17215).
Alexander Kornienko
alexfh at google.com
Tue Oct 8 18:25:00 PDT 2013
PTAL
================
Comment at: lib/Format/Format.cpp:685
@@ -679,3 +684,3 @@
unsigned WhitespaceLength = TrailingWhitespace;
- while (FormatTok->Tok.is(tok::unknown)) {
+ while (isWhitespaceToken(*FormatTok)) {
for (int i = 0, e = FormatTok->TokenText.size(); i != e; ++i) {
----------------
Daniel Jasper wrote:
> I'd probably inline this function. It is used only once and I find it important at this point to know that it does not look into FormatTok's content. Not quite sure whether it is better, though.
Done.
================
Comment at: lib/Format/TokenAnnotator.cpp:575
@@ +574,3 @@
+ Current.Type = TT_ImplicitStringLiteral;
+ if (Current.Type != TT_Unknown)
+ return;
----------------
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.
================
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;
----------------
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.
http://llvm-reviews.chandlerc.com/D1858
More information about the cfe-commits
mailing list