[PATCH] D76793: [Matrix] Implement + and - operators for MatrixType.

[Florian Hahn via Phabricator via cfe-commits]

fhahn marked 2 inline comments as done.
fhahn added inline comments.


================
Comment at: clang/lib/Sema/SemaOverload.cpp:8587
+        if (S.Context.hasSameType(M1, M2))
+          AddCandidate(M1, M2);
+
----------------
rjmccall wrote:
> I don't think this works if one side or the other has e.g. a templated conversion to a matrix type.  You need to add:
> 
> ```
>    (M1, M1) -> M1
>    (M1, M1.ElementType) -> M1
>    (M2, M2) -> M2    // but don't introduce the same candidate twice if the same type is also in the LHS set)
>    (M2.ElementType, M2) -> M2
> ```
> 
> Test case is something like:
> 
> ```
> struct identmatrix_t {
>   template <class T, unsigned N>
>   operator matrix_type<T, N, N>() const {
>     matrix_type<T, N, N> result = {};
>     for (unsigned i = 0; i != N; ++i) result[i][i] = 1;
>     return result;
>   }
> };
> constexpr identmatrix_t identmatrix = identmatrix();
> 
> ...
> m + identmatrix
> ```
> 
> Also, these operator candidates are only for specific operators, right?  I think you can check what the operator is.
Oh I see, I've updated the code to add the versions as suggested.

> Also, these operator candidates are only for specific operators, right? I think you can check what the operator is.

Yes they are. I had to update addGenericBinaryArithmeticOverloads to pass the Op kind.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76793