[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