[PATCH] D23279: clang-reorder-fields

Ben Craig via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 10 10:59:18 PDT 2016


bcraig added a comment.

In https://reviews.llvm.org/D23279#511266, @omtcyfz wrote:

> In https://reviews.llvm.org/D23279#511202, @bcraig wrote:
>
> > In https://reviews.llvm.org/D23279#511187, @omtcyfz wrote:
> >
> > > In https://reviews.llvm.org/D23279#510234, @alexshap wrote:
> > >
> > > > > I do think that an automated tool will do a better job of changing field orderings in a non-breaking way than most people would, mostly due to initializer lists.
> > > >
> > > >
> > > > yeah, that was the original motivation
> > >
> > >
> > > How does that justify its current state, in which it does breaks codebases with more than 1 TU?
> >
> >
> > Yeah, that aspect probably needs work :).  Accepting a compilation database seems like a good way to correct that.
>
>
> Accepting compilation database doesn't solve the problem completely. Most of the existing Clang-based tools accept compilation database, but they do not pass information between translation units, which is the most important part.


I will agree that compilation databases are a (mostly) necessary, but not sufficient part of the solution.  The refactoring tool will need to be careful about changing the structure definition.  It needs to remember (across TUs) what the old definition was and what the desired definition is, so that it can correctly fix up the uses and initializer lists.  If it saves the files too aggressively, then there will be some serious problems.

So lots of things for @alexshap to work on.


Repository:
  rL LLVM

https://reviews.llvm.org/D23279





More information about the cfe-commits mailing list