[cfe-commits] [PATCH] Initial version of formatting library

[Daniel Jasper]

  Yes, it is going to be a simplified and forgiving C++ parser.

  Your remark about addContinuation makes sense, but I am not sure what the right thing to do is. The important fact about a continuation is that changes within a continuation do not affect changes outside of the continuation. If some style guide wants more line-breaks in certain places, these could also be added (optionally) within the formatContinuation().

  All this being said (and in response to most of your other comments): This is far from being finished. We think we can get to something with (limited) usefulness within the next 3-6 months. However, we'd like to do the development in the open and thus get this committed now and then iterate on it. We know of plenty of things that is does not yet handle correctly and we are going to add tests and fix the behavior.


================
Comment at: lib/Format/ContinuationParser.cpp:171
@@ +170,3 @@
+  Cont.Tokens.push_back(FormatTok);
+  return parseToken();
+}
----------------
Dmitri Gribenko wrote:
> 'return' is not required since return type is void.
Removed.

================
Comment at: lib/Format/ContinuationParser.cpp:129
@@ +128,3 @@
+void ContinuationParser::parseIfThenElse() {
+  if (FormatTok.Tok.getKind() != tok::raw_identifier) abort();
+  nextToken();
----------------
Dmitri Gribenko wrote:
> assert()
Done.

================
Comment at: lib/Format/Format.cpp:45
@@ +44,3 @@
+    State.Indent.push_back(Cont.Level * 2 + 4);
+    for (unsigned i = 1; i < Cont.Tokens.size(); ++i) {
+      bool InsertNewLine = Cont.Tokens[i].NewlinesBefore > 0;
----------------
Dmitri Gribenko wrote:
> ..., e = Cont.Tokens.size(); i != e; ++i ?
Done.

================
Comment at: unittests/Format/FormatTest.cpp:122
@@ +121,3 @@
+      format("if(true)\nif(true)\nif(true)\nf();\n"
+             "else\ng();\nelse\nh();\nelse\ni();", 0, 1));
+  EXPECT_EQ(
----------------
Dmitri Gribenko wrote:
> It is unfortunate that these testcases have duplicated strings.  Here's a trick (you might not like it, it is just a suggestion): add function "AssertionResult checkFormat(char *)".  It will first strip whitespace from the string, pass it to format() and compare results.  This will not work for code with comments, but there will be few of those testcases.
> 
Will keep that in mind, but I don't think it is strongly better, so I don't see a reason to change it now.


http://llvm-reviews.chandlerc.com/D80