[PATCH] D23279: clang-reorder-fields

Daniel Jasper via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 22 18:51:06 PDT 2016


djasper added a comment.

I think quite a bit of the complexity in this patch stems from the two different ways to format the input (defining the field order or alternatively defining the new index of specific fields). Do we really need both? If not, I'd remove one of them for now (likely the index-based one). If we do need both, I'd still remove the index-based version from ReorderFieldsAction and instead provide a helper that converts from one to the other. Or maybe we should even go one step further and move the entire name-to-index-mapping into helper functions and only hand an index-to-index mapping into ReorderFieldsAction. That way, it doesn't need a lot of its error handing. (All of these thoughts are based on the single responsibility principle).


Repository:
  rL LLVM

https://reviews.llvm.org/D23279





More information about the cfe-commits mailing list