[PATCH] D23279: clang-reorder-fields

Alexander Shaposhnikov via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 10 11:37:00 PDT 2016


alexshap added a comment.

0. the tool relies on the compilation database from day one (like the other clang tools using tooling::RefactoringTool)

1. regarding TUs:.

A. just in case how it works: it uses the full name of record to find the definition, then it changes the definition, then it finds all the 
places which needs to be changed (since it's v0 the support is limited - now we handle only C-like structs and visit only InitListExprs).
if you have LibA with public header Foo.h which contain the struct Foo (in particular the layout of Foo is in the ABI of LibA)
and you have LibB in your project which uses struct Foo (and relies on the layout of Foo) - yes, LibB also needs to be changed and rebuilt,
it can be done using this tool - you need to pass all the source files which use Foo to clang-reorder-fields. Yes, certainly there are more complex cases and more interesting dependencies on the layout which are not handled by this tool, i will not list them, the list is too long and i am sure i am not aware of all the cases. The simplest example: if you have Foo2 in the same library or in a separate (it doesn't matter) with exactly the same layout and the code looks like this: Foo* ptr; Foo2* ptr2 = reinterpret_case<Foo2*>(ptr); then yes, obviously it will not work anymore. However as i said above - it doesn't matter which tool you have used to reorder fields - is it your favorite IDE, are you doing it manually - it doesn't matter - if you are making a breaking change you need to understand what you are doing, how it works and what the consequences are.

B. another illustration: the same setup: you have LibA with public header PublicStructs.h which contain the struct Foo and there exists LibB which uses struct Foo and you have decided to rename your struct Foo to Bar and did it only in LibA. In this case the **build** of LibB might get broken and yes, LibB also might need changes/adjustments.

C. having said that, there are also a lot of other cases (for example when you want to change 
**SomeNonABIBreakingNonPublicInAnonymousNamespaceInternalNotMentionedInAnyPublicHeaderAtAll_Foo class**, 
you understand what you are doing and you can guarantee that it doesn't break the ABI) where it seems to me this tool can be useful and more convenient (can save some time) in comparison with doing it by hands. 
Maybe klimek@ or compnerd@ can comment on this to clarify the vision/expectations of clang-based tools.


Repository:
  rL LLVM

https://reviews.llvm.org/D23279





More information about the cfe-commits mailing list