rjmccall added inline comments.

================
Comment at: clang/lib/Sema/SemaExpr.cpp:11989
+  else
+    return false;
+
----------------
fhahn wrote:
> rjmccall wrote:
> > I would suggest checking some preconditions and then just calling 
> > `PrepareScalarCast`.
> > 
> > You should allow implicit conversions from class types, which somewhat 
> > surprisingly I'm not sure we have a convenient method for, but which you 
> > can find workable code for in `ConvertForConditional` in SemaExprCXX.cpp.  
> > Test case is `struct DoubleWrapper { operator double(); };`, and you should 
> > test using that even when the element type isn't a double.
> > 
> > In SemaOverload, you should add builtin candidates for these operators if 
> > one operand or the other is a matrix type.  Basically:
> > 
> > 1. Collect matrix types in `AddTypesConvertedFrom`
> > 2. For each matrix type `M` on the LHS, add candidates for `(M, M) -> M` 
> > and `(M, E) -> M`, and then analogously on the RHS.  Although you might 
> > need to avoid adding redundant candidates if the same type shows up on both 
> > sides.
> Thank you very much John, I hope I managed to update the patch properly 
> according to your comments!
Does it work to just do the initialization step instead of separately checking 
for an arithmetic type?  That code is definitely capable of doing arithmetic 
conversions.


================
Comment at: clang/lib/Sema/SemaOverload.cpp:9173
       OpBuilder.addBinaryPlusOrMinusPointerOverloads(Op);
-      OpBuilder.addGenericBinaryArithmeticOverloads();
+      OpBuilder.addGenericBinaryArithmeticOverloads(Op);
     }
----------------
Maybe you should just extract your new code into its own method and call it 
here.  That might fit in a bit cleaner to the design, since unlike the vector 
case you're not planning on uniformly having element-wise versions of all of 
the operators.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76793



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to