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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
