[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D53738#1278528, @ebevhan wrote:

> In https://reviews.llvm.org/D53738#1278078, @rjmccall wrote:
>
> > I don't think we should add *types* just for this, but if you need to make a different kind of `BinaryOperator` that represents that the semantics aren't quite the same (the same way that the compound assignments have their own subclass), that seems natural to me.
>
>
> I don't know if adding a new operator kind was what I had in mind. The operator hasn't changed, it's still a normal binary operator. Compound assignments are their own operator with its own syntax; it doesn't really strike me as the same thing.


Sorry for being unclear.  I wasn't suggesting adding a new `BinaryOperatorKind`, I was suggesting making a subclass of `BinaryOperator` that stored the extra information you'd like to store.

> The important difference would be that we separate the semantics of performing the conversion and the operation. I understand that adding a new type could be a bit more involved than baking the conversion into the operator, but I really do not enjoy seeing so much implicit, implementation-specific logic encapsulated in the same AST element. Anyone who wants to look at BinaryOperators with fixed-point types will need to know all of the details of how the finding the common type and conversion is done, rather than simply "it does an addition".

It's not just that adding a new type is "involved", it's that it's a bad solution.  Those types can't actually be expressed in the language, and changing the type system to be able to express them is going to lead to a lot of pain elsewhere.


Repository:
  rC Clang

https://reviews.llvm.org/D53738