aaron.ballman added inline comments.
Herald added a subscriber: StephenFan.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:2615
QualType ArgTy = TheCall->getArg(0)->getType();
- QualType EltTy = ArgTy;
-
- if (auto *VecTy = EltTy->getAs<VectorType>())
- EltTy = VecTy->getElementType();
- if (!EltTy->isFloatingType()) {
- Diag(TheCall->getArg(0)->getBeginLoc(),
- diag::err_builtin_invalid_arg_type)
- << 1 << /* float ty*/ 5 << ArgTy;
-
+ if (checkFPMathBuiltinElementType(*this, TheCall->getArg(0)->getBeginLoc(),
+ ArgTy, 1))
----------------
arsenm wrote:
> bob80905 wrote:
> > This change appears to allow more types (such as integers) as args for this
> > set of builtins in the above cases, where before the behavior was to just
> > allow floats.
> > Is this intended behavior?
> >
> No? test/Sema/builtins-elementwise-math.c already checks all of these reject
> integers and passes
As best I can tell, it actually allows *less* types. The old code was calling
`!EltTy->isFloatingType()` and the new code is calling
`!EltTy->isRealFloatingType()`, so the old code would allow a complex float
while the new code prohibits it. Is that intentional?
We should add some explicit test coverage for how these builtins work with
complex types.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:17761-17766
+ for (int I = 0; I < 3; ++I) {
+ ExprResult Converted = UsualUnaryConversions(TheCall->getArg(I));
+ if (Converted.isInvalid())
+ return true;
+ Args[I] = Converted.get();
+ }
----------------
This will cause conversions to happen *before* we check whether the types are
the same; is that expected? e.g., it seems like this would allow you to pass a
float and a double and thanks to the magic of usual unary conversions they both
come out as double and thus don't get diagnosed.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:17775-17785
+ for (int I = 1; I < 3; ++I) {
+ if (Args[0]->getType().getCanonicalType() !=
+ Args[I]->getType().getCanonicalType()) {
+ return Diag(Args[0]->getBeginLoc(),
+ diag::err_typecheck_call_different_arg_types)
+ << Args[0]->getType() << Args[I]->getType();
+ }
----------------
These two loops can be merged together, right?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140992/new/
https://reviews.llvm.org/D140992
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
