[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