fhahn added inline comments.
================ Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11337 +def err_builtin_float_invalid_arg_type: Error < + "%ordinal0 argument must be a " ---------------- There's no need to add a new error message here. `err_builtin_invalid_arg_type` already uses `%select` to support different types in the message. You can just add a new option to the `%select{}` (done by `|new option`). You need to adjust the index passed to `Diag()` accordingly. ================ Comment at: clang/lib/Sema/SemaChecking.cpp:16721 +bool Sema::SemaBuiltinElementwiseMathFloatArg(CallExpr *TheCall) { + if (checkArgCount(*this, TheCall, 1)) ---------------- fhahn wrote: > If I understand correctly, this is similar to > `SemaBuiltinElementwiseMathOneArg`, but with the additional restriction to > only allow floating point element types? > > Do you think it would be possible to extend > `SemaBuiltinElementwiseMathOneArg` to handle this case directly, without > needing to duplicate most of the other logic? The latest version s till duplicates most of the logic unfortunately. I think it would be good to allow the caller of `SemaBuiltinElementwiseMathOneArg` to specify additional restrictions on the argument type. This could be done by adding an extra callback argument, like below: ``` -bool Sema::SemaBuiltinElementwiseMathOneArg(CallExpr *TheCall) { +bool Sema::SemaBuiltinElementwiseMathOneArg( + CallExpr *TheCall, + std::function<bool(QualType ArgTy, SourceLocation ArgLoc)> + ExtraCheckArgTy) { if (checkArgCount(*this, TheCall, 1)) return true; @@ -16707,16 +16734,10 @@ bool Sema::SemaBuiltinElementwiseMathOneArg(CallExpr *TheCall) { TheCall->setArg(0, A.get()); QualType TyA = A.get()->getType(); - if (checkMathBuiltinElementType(*this, ArgLoc, TyA)) + if (checkMathBuiltinElementType(*this, ArgLoc, TyA) || + ExtraCheckArgTy(TyA, ArgLoc)) return true; - QualType EltTy = TyA; - if (auto *VecTy = EltTy->getAs<VectorType>()) - EltTy = VecTy->getElementType(); - if (EltTy->isUnsignedIntegerType()) - return Diag(ArgLoc, diag::err_builtin_invalid_arg_type) - << 1 << /*signed integer or float ty*/ 3 << TyA; - TheCall->setType(TyA); return false; } ``` At the call site, it would look something like ``` - if (SemaBuiltinElementwiseMathOneArg(TheCall)) + if (SemaBuiltinElementwiseMathOneArg( + TheCall, [this](QualType ArgTy, SourceLocation ArgLoc) -> bool { + QualType EltTy = ArgTy; + if (auto *VecTy = EltTy->getAs<VectorType>()) + EltTy = VecTy->getElementType(); + if (EltTy->isUnsignedIntegerType()) + return Diag(ArgLoc, diag::err_builtin_invalid_arg_type) + << 1 << /*signed integer or float ty*/ 3 << ArgTy; + return false; + })) return ExprError(); break; ``` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114688/new/ https://reviews.llvm.org/D114688 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits