Forbid returning pointers and atomics from functions.

Introduce a new `TypeFlags::CONSTRUCTIBLE` flag, corresponding to WGSL's "constructible types". Set this on the appropriate types. Check for this flag on function return types.
gfx-rs · May 1, 2022 · 4346d13 · 4346d13
1 parent fae939f
commit 4346d13
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 6 deletions.
diff --git a/src/valid/function.rs b/src/valid/function.rs
@@ -86,6 +86,8 @@ pub enum FunctionError {
     },
     #[error("Argument '{name}' at index {index} has a type that can't be passed into functions.")]
     InvalidArgumentType { index: usize, name: String },
+    #[error("The function's given return type cannot be returned from functions")]
+    NonConstructibleReturnType,
     #[error("Argument '{name}' at index {index} is a pointer of space {space:?}, which can't be passed into functions.")]
     InvalidArgumentPointerSpace {
         index: usize,
@@ -894,6 +896,17 @@ impl super::Validator {
             }
         }
 
+        #[cfg(feature = "validate")]
+        if let Some(ref result) = fun.result {
+            if !self.types[result.ty.index()]
+                .flags
+                .contains(super::TypeFlags::CONSTRUCTIBLE)
+            {
+                return Err(FunctionError::NonConstructibleReturnType
+                    .with_span_handle(result.ty, &module.types));
+            }
+        }
+
         self.valid_expression_set.clear();
         self.valid_expression_list.clear();
         for (handle, expr) in fun.expressions.iter() {

diff --git a/src/valid/type.rs b/src/valid/type.rs
@@ -52,6 +52,15 @@ bitflags::bitflags! {
 
         /// This type can be passed as a function argument.
         const ARGUMENT = 0x40;
+
+        /// A WGSL [constructible] type.
+        ///
+        /// The constructible types are scalars, vectors, matrices, fixed-size
+        /// arrays of constructible types, and structs whose members are all
+        /// constructible.
+        ///
+        /// [constructible]: https://gpuweb.github.io/gpuweb/wgsl/#constructible
+        const CONSTRUCTIBLE = 0x80;
     }
 }
 
@@ -217,7 +226,8 @@ impl super::Validator {
                         | TypeFlags::COPY
                         | TypeFlags::IO_SHAREABLE
                         | TypeFlags::HOST_SHAREABLE
-                        | TypeFlags::ARGUMENT,
+                        | TypeFlags::ARGUMENT
+                        | TypeFlags::CONSTRUCTIBLE,
                     width as u32,
                 )
             }
@@ -232,7 +242,8 @@ impl super::Validator {
                         | TypeFlags::COPY
                         | TypeFlags::IO_SHAREABLE
                         | TypeFlags::HOST_SHAREABLE
-                        | TypeFlags::ARGUMENT,
+                        | TypeFlags::ARGUMENT
+                        | TypeFlags::CONSTRUCTIBLE,
                     count * (width as u32),
                 )
             }
@@ -251,7 +262,8 @@ impl super::Validator {
                         | TypeFlags::COPY
                         | TypeFlags::IO_SHAREABLE
                         | TypeFlags::HOST_SHAREABLE
-                        | TypeFlags::ARGUMENT,
+                        | TypeFlags::ARGUMENT
+                        | TypeFlags::CONSTRUCTIBLE,
                     count * (width as u32),
                 )
             }
@@ -462,8 +474,10 @@ impl super::Validator {
                     }
                 };
 
-                let base_mask =
-                    TypeFlags::COPY | TypeFlags::HOST_SHAREABLE | TypeFlags::IO_SHAREABLE;
+                let base_mask = TypeFlags::COPY
+                    | TypeFlags::HOST_SHAREABLE
+                    | TypeFlags::IO_SHAREABLE
+                    | TypeFlags::CONSTRUCTIBLE;
                 TypeInfo {
                     flags: TypeFlags::DATA | (base_info.flags & base_mask) | sized_flag,
                     uniform_layout,
@@ -481,7 +495,8 @@ impl super::Validator {
                         | TypeFlags::COPY
                         | TypeFlags::HOST_SHAREABLE
                         | TypeFlags::IO_SHAREABLE
-                        | TypeFlags::ARGUMENT,
+                        | TypeFlags::ARGUMENT
+                        | TypeFlags::CONSTRUCTIBLE,
                     1,
                 );
                 ti.uniform_layout = Ok(Some(UNIFORM_MIN_ALIGNMENT));

diff --git a/tests/wgsl-errors.rs b/tests/wgsl-errors.rs
@@ -987,6 +987,43 @@ fn invalid_functions() {
         })
         if function_name == "unacceptable_ptr_space" && argument_name == "arg"
     }
+
+    check_validation_error! {
+        "
+        struct AFloat {
+          said_float: f32
+        };
+        @group(0) @binding(0)
+        var<storage> float: AFloat;
+
+        fn return_pointer() -> ptr<storage, f32> {
+           return &float.said_float;
+        }
+        ":
+        Err(naga::valid::ValidationError::Function {
+            name: function_name,
+            error: naga::valid::FunctionError::NonConstructibleReturnType,
+            ..
+        })
+        if function_name == "return_pointer"
+    }
+
+    check_validation_error! {
+        "
+        @group(0) @binding(0)
+        var<storage> atom: atomic<u32>;
+
+        fn return_atomic() -> atomic<u32> {
+           return atom;
+        }
+        ":
+        Err(naga::valid::ValidationError::Function {
+            name: function_name,
+            error: naga::valid::FunctionError::NonConstructibleReturnType,
+            ..
+        })
+        if function_name == "return_atomic"
+    }
 }
 
 #[test]