[PATCH] D23279: clang-reorder-fields

[Daniel Jasper via cfe-commits]

djasper added a comment.

Ben: I am happy to have this as a separate tool until then. But if we go that route, let's add very explicit comments about this so that people don't start depending on it and we can actually remove it as easily as possible.

Alex: I don't think there will be a significant performance difference between implementing this as recursive AST visitor or implementing this with matchers. Even if you have multiple matchers, a single run over the AST would suffice. If there is a performance difference, that's probably something we should fix. It's hard to make an estimate on which version would be more concise without actually comparing both versions (I would bet on the AST matcher version to be more concise). However, an equally important point is that a single way to find something in the AST is easier to understand and maintain then having two ways to do it in the same tool.


================
Comment at: test/clang-reorder-fields/CStructAmbiguousName.cpp:6
@@ +5,3 @@
+struct Foo {
+  int x;    // CHECK: int x;
+  double y; // CHECK: double y;
----------------
Have you thought about how to handle comments that surround these?


Repository:
  rL LLVM

https://reviews.llvm.org/D23279