[PATCH] D48716: [clang-format] Fix counting parameters/arguments for ObjC

Jacek Olesiak via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 2 02:54:32 PDT 2018


jolesiak marked 6 inline comments as done.
jolesiak added a comment.

In https://reviews.llvm.org/D48716#1146796, @benhamilton wrote:

> > Count selector parts also for method declarations.
>
> What bug does this fix? Can you add a test which breaks before this change and is fixed by this change?


Sorry for the confusion.

General comment to changes https://reviews.llvm.org/D48716, https://reviews.llvm.org/D48718, https://reviews.llvm.org/D48719, https://reviews.llvm.org/D48720 (which are the split of https://reviews.llvm.org/D48352):
These changes are separate, in the sense that they fix different issues. However, they should be chained as every change (apart from the first one) is based on previous ones: https://reviews.llvm.org/D48716 -> https://reviews.llvm.org/D48718 -> https://reviews.llvm.org/D48719 -> https://reviews.llvm.org/D48720. I don't know how to chain them in Phabricator (I should probably leave some comments about what every change is based on though).

https://reviews.llvm.org/D48716 fixes the mechanism of counting parameters. This is an internal change though and doesn't influence formatting on its own (at the current state). Its lack would be visible after applying https://reviews.llvm.org/D48719. Therefore, I'm not aware of any formatting test that fails before applying this change and succeeds after. As far as I know internal functions/methods are not tested at all. Thus, the tests are part of https://reviews.llvm.org/D48719.

This is why initially I put everything into one change: https://reviews.llvm.org/D48352, but I agree that it was not readable at all.


Repository:
  rC Clang

https://reviews.llvm.org/D48716





More information about the cfe-commits mailing list