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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits