fhahn added inline comments.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:11989
+ else
+ return false;
+
----------------
rjmccall wrote:
> 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.
Yes it does. Much simpler, thanks!
================
Comment at: clang/lib/Sema/SemaOverload.cpp:9173
OpBuilder.addBinaryPlusOrMinusPointerOverloads(Op);
- OpBuilder.addGenericBinaryArithmeticOverloads();
+ OpBuilder.addGenericBinaryArithmeticOverloads(Op);
}
----------------
rjmccall wrote:
> 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.
Sounds good, thanks!
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
