[PATCH] D19804: Make clang-format cleaner remove redundant commas/colons in constructor initializer list.

Eric Liu via cfe-commits cfe-commits at lists.llvm.org
Mon May 9 03:20:22 PDT 2016


ioeric added inline comments.

================
Comment at: lib/Format/Format.cpp:1821
@@ +1820,3 @@
+      if (Line->Affected)
+        checkConstructorInitList(*Line);
+    }
----------------
djasper wrote:
> Why are we restricting this to constructor initializers? I think we should directly be more generic and clean up different lists. Also, as an idea? Could we make this very generic and implement a function that analyzes a line for a specific sequence of tokens? E.g., I would assume that then the current checkConstructorInitList() could be written as:
> 
>   cleanupLeft(tok::comma, tok::comma);
>   cleanupLeft(tok::comma, tok::l_brace);
>   cleanupRight(tok::colon, tok::comma);
>   cleanupLeft(tok::colon, tok::l_brace);
> 
> With cleanupLeft/Right meaning: Find this sequence of tokens (ignoring comments) and then clean up the left or the right side.
> 
> Not sure about the exact names of functions etc. What do you think?
I think having a generic `cleanupLeft(tok::comma, tok::comma)` is a great idea; however, the other three functions seem a little too generic since they'd probably only be used in constructor initializers? E.g. an expression like `condition ? something : { list }` would be a false positive for `cleanupLeft(tok::colon, tok::l_brace)`, and `[{...}, {...}]` might be a false positive for `cleanupLeft(tok::comma, tok::l_brace)`. Also, it seems to me that `cleanupRight(tok::colon, tok::comma)` would only happen in constructor initializers?

I think a mixed solution might work as well. For example, we can run `cleanupLeft(tok::comma, tok::comma)` across all tokens in the code, and then for constructor initializers specifically, we handle redundant colon and trailing comma. What do you think?

As for comments, we can probably handle them after all cleanup is done.


http://reviews.llvm.org/D19804





More information about the cfe-commits mailing list