[clang] [clang] Restrict use of scalar types in vector builtins (PR #119423)

[Fraser Cormack via cfe-commits]

================
@@ -14604,57 +14611,63 @@ bool Sema::PrepareBuiltinElementwiseMathOneArgCall(CallExpr *TheCall) {
 }
 
 bool Sema::BuiltinElementwiseMath(CallExpr *TheCall, bool FPOnly) {
-  QualType Res;
-  if (BuiltinVectorMath(TheCall, Res, FPOnly))
-    return true;
-  TheCall->setType(Res);
-  return false;
+  if (auto Res = BuiltinVectorMath(TheCall, FPOnly); Res.has_value()) {
+    TheCall->setType(*Res);
+    return false;
+  }
+  return true;
 }
 
 bool Sema::BuiltinVectorToScalarMath(CallExpr *TheCall) {
-  QualType Res;
-  if (BuiltinVectorMath(TheCall, Res))
+  std::optional<QualType> Res = BuiltinVectorMath(TheCall);
+  if (!Res)
     return true;
 
-  if (auto *VecTy0 = Res->getAs<VectorType>())
+  if (auto *VecTy0 = (*Res)->getAs<VectorType>())
     TheCall->setType(VecTy0->getElementType());
   else
-    TheCall->setType(Res);
+    TheCall->setType(*Res);
 
   return false;
 }
 
-bool Sema::BuiltinVectorMath(CallExpr *TheCall, QualType &Res, bool FPOnly) {
+std::optional<QualType> Sema::BuiltinVectorMath(CallExpr *TheCall,
+                                                bool FPOnly) {
   if (checkArgCount(TheCall, 2))
-    return true;
+    return std::nullopt;
 
-  ExprResult A = TheCall->getArg(0);
-  ExprResult B = TheCall->getArg(1);
-  // Do standard promotions between the two arguments, returning their common
-  // type.
-  Res = UsualArithmeticConversions(A, B, TheCall->getExprLoc(), ACK_Comparison);
-  if (A.isInvalid() || B.isInvalid())
-    return true;
+  checkEnumArithmeticConversions(TheCall->getArg(0), TheCall->getArg(1),
+                                 TheCall->getExprLoc(), ACK_Comparison);
----------------
frasercrmck wrote:

Can you clarify what you mean here, sorry? I assume you mean just in the context of these vector builtins. This call looks like it's just checking the following:

```
  // C++2a [expr.arith.conv]p1:
  //   If one operand is of enumeration type and the other operand is of a
  //   different enumeration type or a floating-point type, this behavior is
  //   deprecated ([depr.arith.conv.enum]).
  //
  // Warn on this in all language modes. Produce a deprecation warning in C++20.
  // Eventually we will presumably reject these cases (in C++23 onwards?).
```

So it's not really whether "enums are allowed", but whether you're allowed to mix different enums, or enums with floating-point values, in binary builtins.

I agree the inconsistency here is not desirable, especially as we're not doing the same for the ternary elementwise builtins.

Should we just forbid this in all binary/ternary vector elementwise builtins?

https://github.com/llvm/llvm-project/pull/119423