================
@@ -2245,6 +2245,36 @@ bool SemaHLSL::CheckBuiltinFunctionCall(unsigned 
BuiltinID, CallExpr *TheCall) {
 
     break;
   }
+  case Builtin::BI__builtin_hlsl_and: {
+    if (SemaRef.checkArgCount(TheCall, 2))
+      return true;
+    if (CheckVectorElementCallArgs(&SemaRef, TheCall))
+      return true;
+
+    // CheckVectorElementCallArgs(...) guarantees both args are the same type.
+    assert(TheCall->getArg(0)->getType() == TheCall->getArg(1)->getType() &&
+           "Both args must be of the same type");
+
+    // check that the arguments are bools or, if vectors,
+    // vectors of bools
+    QualType ArgTy = TheCall->getArg(0)->getType();
+    if (const auto *VecTy = ArgTy->getAs<VectorType>()) {
+      ArgTy = VecTy->getElementType();
+    }
+    if (!getASTContext().hasSameUnqualifiedType(ArgTy,
----------------
farzonl wrote:

> On this point I actually disagree. We should get this PR into a state where 
> its diagnostics are at the high quality we want, then we can work back to the 
> other diagnostics and correct them.

I’d prefer to take the time to plan a cohesive fix that updates all the tests 
rather than introducing an ad hoc solution for `and`, which would likely result 
in another ad hoc fix for the `or` builtin (also scheduled for this sprint).

For me, any work scheduled in the current sprint has a high bar for deviating 
from our established best practices. This is especially important because my 
primary goal is to ensure a smooth and predictable landing for the intrinsics 
workstream, as it plays a key role in onboarding new contributors.

Next week, I’d like to discuss a change that would either:

1. Consolidate `CheckArgTypeIsCorrect` and `CheckScalarOrVector`.
2. Modify `CheckArgTypeIsCorrect` to use the 
`err_typecheck_expect_scalar_or_vector` diagnostic and stop constructing the 
expected vector type. This would mean we stop printing an expected type with 
the syntactic sugar missing. Additionally, we would update ExpectedType to be a 
QualType vector or ArrayRef ExpectedTypes, allowing us to properly capture 
multiple overloads.

ie go from
```cpp
static bool CheckArgTypeIsCorrect(
    Sema *S, Expr *Arg, QualType ExpectedType,
    llvm::function_ref<bool(clang::QualType PassedType)> Check) {
  QualType PassedType = Arg->getType();
  if (Check(PassedType)) {
    if (auto *VecTyA = PassedType->getAs<VectorType>())
      ExpectedType = S->Context.getVectorType(
          ExpectedType, VecTyA->getNumElements(), VecTyA->getVectorKind());
    S->Diag(Arg->getBeginLoc(), diag::err_typecheck_convert_incompatible)
        << PassedType << ExpectedType << 1 << 0 << 0;
    return true;
  }
  return false;
}
```
to
```cpp
static bool CheckArgTypeIsCorrect(
    Sema *S, Expr *Arg, llvm::SmallVectorImpl<QualType> ExpectedTypes,
    llvm::function_ref<bool(clang::QualType PassedType)> Check) {
  QualType PassedType = Arg->getType();
  if (Check(PassedType)) {
    for (const clang::QualType &ExpectedType : ExpectedTypes)
      S->Diag(TheCall->getArg(0)->getBeginLoc(),
            diag::err_typecheck_expect_scalar_or_vector)
        << ArgType << ExpectedType;

```
My feeling is we can achieve what you want to do here but it would turn out 
better if these type of fixes had time to be designed outside of the PR 
process. I hope this better explains where I'm coming from.


https://github.com/llvm/llvm-project/pull/127098
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to