From df755c3630b387087ad166b604092d2f2d07ecf6 Mon Sep 17 00:00:00 2001
From: Erich Gubler <erichdongubler@gmail.com>
Date: Tue, 13 Dec 2022 17:23:41 -0500
Subject: [PATCH] fixup! refactor: split handle validation into early pass in
 `Validator::validate`

Should implement
<https://github.com/gfx-rs/naga/pull/2090#discussion_r1047736844>.
---
 src/valid/handles.rs | 58 ++++++++++++--------------------------------
 src/valid/mod.rs     | 21 ++--------------
 2 files changed, 18 insertions(+), 61 deletions(-)

diff --git a/src/valid/handles.rs b/src/valid/handles.rs
index 28443c1fec..843d58e62c 100644
--- a/src/valid/handles.rs
+++ b/src/valid/handles.rs
@@ -5,6 +5,9 @@ use crate::{arena::BadHandle, Handle, Range};
 #[cfg(feature = "validate")]
 use crate::{Arena, UniqueArena};
 
+#[cfg(feature = "validate")]
+use super::{TypeError, ValidationError};
+
 #[cfg(feature = "validate")]
 use std::{convert::TryInto, hash::Hash, num::NonZeroU32};
 
@@ -26,9 +29,7 @@ impl super::Validator {
     /// Errors returned by this method are intentionally sparse, for simplicity of implementation.
     /// It is expected that only buggy frontends or fuzzers should ever emit IR that fails this
     /// validation pass.
-    pub(super) fn validate_module_handles(
-        module: &crate::Module,
-    ) -> Result<(), HandleValidationError> {
+    pub(super) fn validate_module_handles(module: &crate::Module) -> Result<(), ValidationError> {
         let &crate::Module {
             ref constants,
             ref entry_points,
@@ -51,18 +52,13 @@ impl super::Validator {
                             name: _,
                             specialization: _,
                             ref inner,
-                        } = constants
-                            .try_get(constant)
-                            .map_err(InvalidHandleError::BadHandle)
-                            .map_err(HandleValidationError::InvalidHandle)?;
+                        } = constants.try_get(constant)?;
                         if !matches!(inner, &crate::ConstantInner::Scalar { .. }) {
-                            return Err(HandleValidationError::InvalidArraySize(
-                                InvalidArraySizeError {
-                                    ty: this_handle,
-                                    ty_name: name.clone(),
-                                    depends_on: constant,
-                                },
-                            ));
+                            return Err(ValidationError::Type {
+                                handle: this_handle,
+                                name: name.clone().unwrap_or_default(),
+                                source: TypeError::InvalidArraySizeConstant(constant),
+                            });
                         }
                     }
                     crate::ArraySize::Dynamic => (),
@@ -489,42 +485,27 @@ impl super::Validator {
     }
 }
 
-#[derive(Clone, Debug)]
-pub(super) enum HandleValidationError {
-    InvalidHandle(InvalidHandleError),
-    InvalidArraySize(InvalidArraySizeError),
-}
-
-impl From<InvalidHandleError> for HandleValidationError {
-    fn from(source: InvalidHandleError) -> Self {
-        Self::InvalidHandle(source)
-    }
-}
-
-impl From<BadHandle> for HandleValidationError {
+#[cfg(feature = "validate")]
+impl From<BadHandle> for ValidationError {
     fn from(source: BadHandle) -> Self {
         Self::InvalidHandle(source.into())
     }
 }
 
-impl From<FwdDepError> for HandleValidationError {
+#[cfg(feature = "validate")]
+impl From<FwdDepError> for ValidationError {
     fn from(source: FwdDepError) -> Self {
         Self::InvalidHandle(source.into())
     }
 }
 
-impl From<BadRangeError> for HandleValidationError {
+#[cfg(feature = "validate")]
+impl From<BadRangeError> for ValidationError {
     fn from(source: BadRangeError) -> Self {
         Self::InvalidHandle(source.into())
     }
 }
 
-impl From<InvalidArraySizeError> for HandleValidationError {
-    fn from(source: InvalidArraySizeError) -> Self {
-        Self::InvalidArraySize(source)
-    }
-}
-
 #[derive(Clone, Debug, thiserror::Error)]
 pub enum InvalidHandleError {
     #[error(transparent)]
@@ -535,13 +516,6 @@ pub enum InvalidHandleError {
     BadRange(#[from] BadRangeError),
 }
 
-#[derive(Clone, Debug)]
-pub(super) struct InvalidArraySizeError {
-    pub(super) ty: Handle<crate::Type>,
-    pub(super) ty_name: Option<String>,
-    pub(super) depends_on: Handle<crate::Constant>,
-}
-
 #[derive(Clone, Debug, thiserror::Error)]
 #[error(
     "{subject:?} of kind depends on {depends_on:?} of kind {depends_on_kind}, which has not been \
diff --git a/src/valid/mod.rs b/src/valid/mod.rs
index e129adc706..fa5e877e40 100644
--- a/src/valid/mod.rs
+++ b/src/valid/mod.rs
@@ -32,7 +32,7 @@ pub use function::{CallError, FunctionError, LocalVariableError};
 pub use interface::{EntryPointError, GlobalVariableError, VaryingError};
 pub use r#type::{Disalignment, TypeError, TypeFlags};
 
-use self::handles::{HandleValidationError, InvalidArraySizeError, InvalidHandleError};
+use self::handles::InvalidHandleError;
 
 bitflags::bitflags! {
     /// Validation flags.
@@ -159,7 +159,7 @@ pub enum ConstantError {
 #[derive(Clone, Debug, thiserror::Error)]
 pub enum ValidationError {
     #[error(transparent)]
-    InvalidHandle(InvalidHandleError),
+    InvalidHandle(#[from] InvalidHandleError),
     #[error(transparent)]
     Layouter(#[from] LayoutError),
     #[error("Type {handle:?} '{name}' is invalid")]
@@ -196,23 +196,6 @@ pub enum ValidationError {
     Corrupted,
 }
 
-impl From<HandleValidationError> for ValidationError {
-    fn from(e: HandleValidationError) -> Self {
-        match e {
-            HandleValidationError::InvalidHandle(e) => Self::InvalidHandle(e),
-            HandleValidationError::InvalidArraySize(InvalidArraySizeError {
-                ty,
-                ty_name,
-                depends_on,
-            }) => Self::Type {
-                handle: ty,
-                name: ty_name.unwrap_or_default(),
-                source: TypeError::InvalidArraySizeConstant(depends_on),
-            },
-        }
-    }
-}
-
 impl crate::TypeInner {
     #[cfg(feature = "validate")]
     const fn is_sized(&self) -> bool {