From d223bb582f58d28e36e9cf34f79b6644f5649603 Mon Sep 17 00:00:00 2001
From: Andy Leiserson <aleiserson@mozilla.com>
Date: Tue, 18 Mar 2025 16:08:15 -0700
Subject: [PATCH] [naga] Disallow logical operators `&&` and `||` on vectors

Fixes #6856
---
 CHANGELOG.md                        |  1 +
 naga/src/proc/constant_evaluator.rs |  7 +++--
 naga/src/proc/typifier.rs           | 17 +++++++++--
 naga/tests/naga/wgsl_errors.rs      | 45 +++++++++++++++++++++++++++++
 4 files changed, 65 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1537ad4339..a08fecdcfc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -283,6 +283,7 @@ By @syl20bnr in [#7326](https://github.com/gfx-rs/wgpu/pull/7326).
 - Allows override-sized arrays to resolve to the same size without causing the type arena to panic. By @KentSlaney in [#7082](https://github.com/gfx-rs/wgpu/pull/7082).
 - Allow abstract types to be used for WGSL switch statement selector and case selector expressions. By @jamienicol in [#7250](https://github.com/gfx-rs/wgpu/pull/7250).
 - Apply automatic conversions to `let` declarations, and accept `vecN()` as a constructor for vectors (in any context). By @andyleiserson in [#7367](https://github.com/gfx-rs/wgpu/pull/7367).
+- The `&&` and `||` operators are no longer allowed on vectors. By @andyleiserson in [#7368](https://github.com/gfx-rs/wgpu/pull/7368).
 
 #### General
 
diff --git a/naga/src/proc/constant_evaluator.rs b/naga/src/proc/constant_evaluator.rs
index a9e6c7fb12..f0085a32ee 100644
--- a/naga/src/proc/constant_evaluator.rs
+++ b/naga/src/proc/constant_evaluator.rs
@@ -2261,10 +2261,13 @@ impl<'a> ConstantEvaluator<'a> {
             | BinaryOperator::And
             | BinaryOperator::ExclusiveOr
             | BinaryOperator::InclusiveOr
-            | BinaryOperator::LogicalAnd
-            | BinaryOperator::LogicalOr
             | BinaryOperator::ShiftLeft
             | BinaryOperator::ShiftRight => left_ty,
+
+            BinaryOperator::LogicalAnd | BinaryOperator::LogicalOr => {
+                // Not supported on vectors
+                return Err(ConstantEvaluatorError::InvalidBinaryOpArgs);
+            }
         };
 
         let components = components
diff --git a/naga/src/proc/typifier.rs b/naga/src/proc/typifier.rs
index 3bb47d5440..535e525409 100644
--- a/naga/src/proc/typifier.rs
+++ b/naga/src/proc/typifier.rs
@@ -592,9 +592,8 @@ impl<'a> ResolveContext<'a> {
                 | crate::BinaryOperator::Less
                 | crate::BinaryOperator::LessEqual
                 | crate::BinaryOperator::Greater
-                | crate::BinaryOperator::GreaterEqual
-                | crate::BinaryOperator::LogicalAnd
-                | crate::BinaryOperator::LogicalOr => {
+                | crate::BinaryOperator::GreaterEqual => {
+                    // These accept scalars or vectors.
                     let scalar = crate::Scalar::BOOL;
                     let inner = match *past(left)?.inner_with(types) {
                         Ti::Scalar { .. } => Ti::Scalar(scalar),
@@ -607,6 +606,18 @@ impl<'a> ResolveContext<'a> {
                     };
                     TypeResolution::Value(inner)
                 }
+                crate::BinaryOperator::LogicalAnd | crate::BinaryOperator::LogicalOr => {
+                    // These accept scalars only.
+                    let bool = Ti::Scalar(crate::Scalar::BOOL);
+                    let ty = past(left)?.inner_with(types);
+                    if *ty == bool {
+                        TypeResolution::Value(bool)
+                    } else {
+                        return Err(ResolveError::IncompatibleOperands(format!(
+                            "{op:?}({ty:?}, _)"
+                        )));
+                    }
+                }
                 crate::BinaryOperator::And
                 | crate::BinaryOperator::ExclusiveOr
                 | crate::BinaryOperator::InclusiveOr
diff --git a/naga/tests/naga/wgsl_errors.rs b/naga/tests/naga/wgsl_errors.rs
index a662a6aa9c..9e85ab5c86 100644
--- a/naga/tests/naga/wgsl_errors.rs
+++ b/naga/tests/naga/wgsl_errors.rs
@@ -3155,6 +3155,51 @@ fn matrix_vector_pointers() {
     );
 }
 
+#[test]
+fn vector_logical_ops() {
+    // Const context
+    check(
+        "const and = vec2(true, false) && vec2(false, false);",
+        r###"error: Cannot apply the binary op to the arguments
+  ┌─ wgsl:1:13
+  │
+1 │ const and = vec2(true, false) && vec2(false, false);
+  │             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ see msg
+
+"###,
+    );
+
+    check(
+        "const or = vec2(true, false) || vec2(false, false);",
+        r###"error: Cannot apply the binary op to the arguments
+  ┌─ wgsl:1:12
+  │
+1 │ const or = vec2(true, false) || vec2(false, false);
+  │            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ see msg
+
+"###,
+    );
+
+    // Runtime context
+    check(
+        "fn foo(a: vec2<bool>, b: vec2<bool>) {
+            let y = a && b;
+        }",
+        r#"error: Incompatible operands: LogicalAnd(Vector { size: Bi, scalar: Scalar { kind: Bool, width: 1 } }, _)
+
+"#,
+    );
+
+    check(
+        "fn foo(a: vec2<bool>, b: vec2<bool>) {
+            let y = a || b;
+        }",
+        r#"error: Incompatible operands: LogicalOr(Vector { size: Bi, scalar: Scalar { kind: Bool, width: 1 } }, _)
+
+"#,
+    );
+}
+
 #[test]
 fn issue7165() {
     // Regression test for https://github.com/gfx-rs/wgpu/issues/7165