[PATCH] Correctly handle escaped newlines when the next token starts without a space.

Daniel Jasper djasper at google.com
Fri Apr 11 03:24:18 PDT 2014


  I am generally fine with this, as it seems irrelevant in practice. It seems weird, however, to replace an escaped newline with a newline.


================
Comment at: unittests/Format/FormatTest.cpp:749-750
@@ -748,4 +748,4 @@
 
-  EXPECT_EQ("int i; // single line trailing comment",
+  EXPECT_EQ("int i;\n// single line trailing comment",
             format("int i;\\\n// single line trailing comment"));
 
----------------
I find these tests easier to read if the strings are split after each \n.


Also, while I don't see a practical use here, I don't think simply removing the escaped newline is the right thing to do here. What other purpose can it have, other than being a notion of "the following comment is bound to this"? But if implementing that is extra effort, I don't care.

================
Comment at: unittests/Format/FormatTest.cpp:934
@@ -933,3 +933,3 @@
   verifyFormat("void f() { g(/*aaa=*/x, /*bbb=*/!y); }");
-  EXPECT_EQ("f(aaaaaaaaaaaaaaaaaaaaaaaaa, /* Trailing comment for aa... */\n"
+  EXPECT_EQ("f(aaaaaaaaaaaaaaaaaaaaaaaaa,\n  /* Trailing comment for aa... */\n"
             "  bbbbbbbbbbbbbbbbbbbbbbbbb);",
----------------
Same here.

I think here it is even worse as the test makes no sense whatsoever unless we also change the content of the comment. If we decided to keep this, maybe we should just remove/replace these tests?


http://reviews.llvm.org/D3351

BRANCH
  fix-newlines

ARCANIST PROJECT
  clang






More information about the cfe-commits mailing list