[PATCH] D93419: [TableGen] Add the !substr() bang operator

Paul C. Anagnostopoulos via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 21 05:41:02 PST 2020


Paul-C-Anagnostopoulos added inline comments.


================
Comment at: llvm/include/llvm/TableGen/Record.h:832
 public:
-  enum TernaryOp : uint8_t { SUBST, FOREACH, FILTER, IF, DAG };
+  enum TernaryOp : uint8_t { DAG, FILTER, FOREACH, IF, SUBST, SUBSTR };
 
----------------
jansvoboda11 wrote:
> Paul-C-Anagnostopoulos wrote:
> > jansvoboda11 wrote:
> > > Move unrelated/stylistic changes to a separate patch.
> > I wouldn't say this is unrelated, since it is the enum that contains SUBSTR. The previous sort order was apparently arbitrary.
> I understand.
> 
> However, it's a good practice to put formatting changes (and changes that introduce no functionality change) into separate "NFC" commits.
> This helps to keep the interesting patches short and easy to understand.
> 
> In this case, you can create new NFC patch that reorders the enum cases and rebase this patch on top of the new one.
But then if I revert to the previous order, where should I insert SUBSTR? New ones have not necessarily been inserted at the end of the list. Anyway, I'll revert to the original order.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93419/new/

https://reviews.llvm.org/D93419



More information about the llvm-commits mailing list