This is an archive of the discontinued LLVM Phabricator instance.

[clang-move] Better support enclosing class.
ClosedPublic

Authored by hokein on Oct 7 2016, 7:03 AM.

Details

Summary
  • When moving an outermost enclosing class, all its nested classes should also be moved together.
  • Add a test for not moving nested class.

Diff Detail

Repository
rL LLVM

Event Timeline

hokein updated this revision to Diff 73928.Oct 7 2016, 7:03 AM
hokein retitled this revision from to [clang-move] Better support enclosing class..
hokein updated this object.
hokein added a reviewer: ioeric.
hokein added a subscriber: cfe-commits.
ioeric added inline comments.Oct 7 2016, 7:13 AM
clang-move/ClangMove.cpp
27 ↗(On Diff #73928)

I'm not sure if we really need to limit this to the outer-most class. In the future, we might want to support moving nested class, so I guess this can simply be ofClass? Not sure if the name is right though.

343 ↗(On Diff #73928)

What about static variable of the nested class?

hokein updated this revision to Diff 73976.Oct 7 2016, 12:49 PM
hokein marked 2 inline comments as done.

support moving static member of nested class when moving its enclosing class.

hokein added inline comments.Oct 7 2016, 12:52 PM
clang-move/ClangMove.cpp
27 ↗(On Diff #73928)

clang-move can't handle nested classes well enough currently, and supporting nested class requires more stuff there, like changing the qualifiers of the method definitions in the nested class (int A::B::f() should be int B::f() when moving nested class A::B). So we'd better to only allow outer-most class at least for now .

ioeric accepted this revision.Oct 8 2016, 7:28 AM
ioeric edited edge metadata.

lg

This revision is now accepted and ready to land.Oct 8 2016, 7:28 AM
hokein updated this revision to Diff 74490.Oct 13 2016, 3:27 AM
hokein edited edge metadata.

Rebase to master.

This revision was automatically updated to reflect the committed changes.