From 25f03960e1fb7162215549fe7fa613d04ee4693b Mon Sep 17 00:00:00 2001 From: Jim Blandy <jimb@red-bean.com> Date: Wed, 9 Apr 2025 12:58:44 -0700 Subject: [PATCH] [naga] Delete `DiagnosticDisplay` impl for for `TypeInner`. Delete the implementation of `core::fmt::Display` for `naga::common::DiagnosticDisplay`, as it is a footgun wherever `Struct` types can arise. Document that it should not be implemented for `TypeInner`. Although it is possible to put a dummy implementation in the way of people adding real implementations, that would also turn attempts to use it into dynamic errors rather than compile-time errors. Make do with a comment for now. --- naga/src/common/diagnostic_display.rs | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/naga/src/common/diagnostic_display.rs b/naga/src/common/diagnostic_display.rs index 06164a536..2713d5a0e 100644 --- a/naga/src/common/diagnostic_display.rs +++ b/naga/src/common/diagnostic_display.rs @@ -1,7 +1,7 @@ //! Displaying Naga IR terms in diagnostic output. use crate::proc::{GlobalCtx, Rule, TypeResolution}; -use crate::{Handle, Scalar, Type, TypeInner}; +use crate::{Handle, Scalar, Type}; #[cfg(any(feature = "wgsl-in", feature = "wgsl-out"))] use crate::common::wgsl::TypeContext; @@ -23,13 +23,16 @@ use core::fmt; /// for a type like `DiagnosticDisplay<(Handle<Type>, GlobalCtx)>`, where /// the [`GlobalCtx`] type provides the necessary context. /// +/// Do not implement this type for [`TypeInner`], as that does not +/// have enough information to display struct types correctly. +/// /// If you only need debugging output, [`DiagnosticDebug`] uses /// easier-to-obtain context types but still does a good enough job /// for logging or debugging. /// /// [`Display`]: core::fmt::Display -/// [`Scalar`]: crate::Scalar /// [`GlobalCtx`]: crate::proc::GlobalCtx +/// [`TypeInner`]: crate::ir::TypeInner /// [`DiagnosticDebug`]: super::DiagnosticDebug /// /// ## Language-sensitive diagnostics @@ -83,23 +86,6 @@ impl fmt::Display for DiagnosticDisplay<(Handle<Type>, GlobalCtx<'_>)> { } } -impl fmt::Display for DiagnosticDisplay<(&TypeInner, GlobalCtx<'_>)> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let (inner, ref ctx) = self.0; - - #[cfg(any(feature = "wgsl-in", feature = "wgsl-out"))] - ctx.write_type_inner(inner, f)?; - - #[cfg(not(any(feature = "wgsl-in", feature = "wgsl-out")))] - { - let _ = ctx; - write!(f, "{inner:?}")?; - } - - Ok(()) - } -} - impl fmt::Display for DiagnosticDisplay<(&str, &Rule, GlobalCtx<'_>)> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let (name, rule, ref ctx) = self.0;