[PATCH] D23279: clang-reorder-fields

[Alexander Shaposhnikov via cfe-commits]

alexshap added a comment.

First of all, many thanks for the comments & proposal of clang-refactor.

1. Regarding high-level project organization:

clang-refactor - that can be a good place for various refactoring techniques
like the existing clang-rename, my simple tool, etc - essentially what Kirill said in the e-mail.
I would be happy to join. 
At the same time - if i can make some progress before this reorganization is introduced - 
that would be helpful, if not - no problem at all.

2. Regarding clang-tidy - it seems to me, that putting this particular tool into PaddingChecker/clang-tidy is far from being perfect: A. In some cases the optimal fields order is not unique, it would be more flexible to let the user choose which order of fields he prefers. B. In some cases someone might want to change the order of fields even if the padding is not affected at all. C. Other concerns mentioned above in your comments

One more thing: 
i am not sure i understood the comment  
"It actually breaks code ... only works while it is in the same TU".
For instance if we have the following project:
/include/Point.h (contains the definition of the struct Point)
/a.cpp (uses Point)
/b.cpp (also uses Point)
/main.cpp (also uses Point)
clang-reorder-fields -i -record-name ::Point -fields-order y,x main.cpp a.cpp b.cpp
works for me.
At the same time i see in my code the following issue (easy to fix):
it will complain here :

  if (auto Err = Replacements[R.getFilePath()].add(R))
     llvm::errs() << "Failed to add replacement, file: " << R.getFilePath()

because the same replacement for the code in header is being added multiple times, 
but actually it doesn't affect the final set of changes and the result is correct (i think we need either to handle this case more carefully or (temporarily) add FIXME and replace llvm::errs with llvm::warns ).


Repository:
  rL LLVM

https://reviews.llvm.org/D23279