[PATCH] D23279: clang-reorder-fields

Ben Craig via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 22 07:34:28 PDT 2016


bcraig added a comment.

I will note that I'm only really looking at the tests, mostly from the perspective of a potential user.

Thanks for getting C++ initializer lists in.

The tests in general look fine.  The omission of cross visibility reordering is fine as an initial step.

Once multi-TU support is in, you'll definitely want a test for that.


================
Comment at: test/clang-reorder-fields/ClassMixedInitialization.cpp:1-3
@@ +1,4 @@
+// RUN: cat %s > %t.cpp
+// RUN: clang-reorder-fields -record-name Foo -fields-order e,x,pi,v,s %t.cpp -i -- -std=c++11
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
----------------
Nit:
This general pattern seems suspicious to me.  It really seems like it should be some variation of "clang-reorder-fields [...] | FileCheck %s", without the cat and sed stuff.  Is there a clang-reorder-fields mode where changes are output into a new file / stdout instead of modifying the file in place?


Repository:
  rL LLVM

https://reviews.llvm.org/D23279





More information about the cfe-commits mailing list