[PATCH] D75237: [APFloat] Overload comparison operators

Ehud Katz via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 6 03:51:12 PST 2020


ekatz added a comment.

Generally it LGTM.
But this change may be split into 2 as well (like the `neg` change).

I know that some times you see patches that change and add a lot of code (in the same patch), while in other cases they split into smaller ones. Both approaches are valid, but I think the better one is to split (if possible); especially in cases (like this) that change apply to files, from separate projects/modules, outside the one in which the new API is added.

So, I suggest split it into 2, where one adds the new operators and tests; and the other changes the //compares// in the different locations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75237





More information about the llvm-commits mailing list