aaron.ballman added inline comments.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:16673-16678
+ Expr *A = TheCall->getArg(0);
+ Expr *B = TheCall->getArg(1);
+ QualType TyA = A->getType();
+ QualType TyB = B->getType();
+
+ if (TyA != TyB)
----------------
fhahn wrote:
> aaron.ballman wrote:
> > fhahn wrote:
> > > aaron.ballman wrote:
> > > > Should these arguments undergo usual conversions (array to pointer
> > > > decay, integer promotions, etc)?
> > > I intentionally went with not performing conversions, because it might
> > > lead to surprising results. If the arguments have different types, it is
> > > not clear to me which type should be chosen to try convert the other one;
> > > e.g. if we have __builtin_elementwise_max(float, int)` should the first
> > > argument be converted to `int` or the second one to `float`?
> > >
> > > Forcing the types to match without conversion seem to make them less
> > > error-prone to use, but I might be missing some general rule to handle
> > > type conflicts here?
> > I'm not certain how these builtins are expected to be used, but if they're
> > likely to be used with literals, I think we may want integer promotions
> > because of that alone.
> >
> > Btw, given that these only have two arguments, it seems like we could use
> > the usual arithmetic conversions to pick a common type the same way as
> > happens for binary operators.
> >
> > If you end up not adding any conversions here, we should have a test case
> > to cover passing in two array objects (which won't decay to pointers).
> > I'm not certain how these builtins are expected to be used, but if they're
> > likely to be used with literals, I think we may want integer promotions
> > because of that alone.
>
> Yes, they should ideally work with literals! I think it should be relatively
> straight-forward to add promotion of literals.
>
> > Btw, given that these only have two arguments, it seems like we could use
> > the usual arithmetic conversions to pick a common type the same way as
> > happens for binary operators.
>
> IIUC this happens in `SemaOverload.cpp`, but I am not sure how builtins would
> hook into the infrastructure there. Are there other builtins that are
> similarly overloaded by any chance?
> Yes, they should ideally work with literals! I think it should be relatively
> straight-forward to add promotion of literals.
I was thinking of the literal being an `int` and the variable being a `short`,
so the literal isn't what needs promotion in that case. That said, I suppose
character literals in C++ would promote from `char` to `int`, so there are some
literals that could promote. That'd be a somewhat amusing test case for C and
C++ both (`__builtin_elementwise_max('a', 1);`).
> IUC this happens in SemaOverload.cpp, but I am not sure how builtins would
> hook into the infrastructure there. Are there other builtins that are
> similarly overloaded by any chance?
`Sema::SemaBuiltinUnorderedCompare()` uses this, for example:
```
ExprResult OrigArg0 = TheCall->getArg(0);
ExprResult OrigArg1 = TheCall->getArg(1);
// Do standard promotions between the two arguments, returning their common
// type.
QualType Res = UsualArithmeticConversions(
OrigArg0, OrigArg1, TheCall->getExprLoc(), ACK_Comparison);
if (OrigArg0.isInvalid() || OrigArg1.isInvalid())
return true;
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111985/new/
https://reviews.llvm.org/D111985
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
