From 033268617785fa7759c5fec222b7c8de2dc3058b Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Fri, 17 Nov 2023 01:40:36 +0100 Subject: [PATCH 1/3] [naga] remove `span` feature --- naga-cli/Cargo.toml | 1 - naga/Cargo.toml | 7 ++-- naga/src/arena.rs | 86 ++++++++------------------------------- naga/src/block.rs | 23 +---------- naga/src/proc/emitter.rs | 1 - naga/src/span.rs | 28 +------------ naga/tests/snapshots.rs | 13 ++---- naga/tests/wgsl-errors.rs | 1 - wgpu-core/Cargo.toml | 2 +- 9 files changed, 27 insertions(+), 135 deletions(-) diff --git a/naga-cli/Cargo.toml b/naga-cli/Cargo.toml index 101aa3f04d..0feca747f1 100644 --- a/naga-cli/Cargo.toml +++ b/naga-cli/Cargo.toml @@ -21,7 +21,6 @@ path = "../naga" features = [ "validate", "compact", - "span", "wgsl-in", "wgsl-out", "glsl-in", diff --git a/naga/Cargo.toml b/naga/Cargo.toml index 9729adf709..a1039044f0 100644 --- a/naga/Cargo.toml +++ b/naga/Cargo.toml @@ -26,10 +26,9 @@ deserialize = ["serde", "bitflags/serde", "indexmap/serde"] arbitrary = ["dep:arbitrary", "bitflags/arbitrary", "indexmap/arbitrary"] spv-in = ["petgraph", "spirv"] spv-out = ["spirv"] -wgsl-in = ["codespan-reporting", "hexf-parse", "termcolor", "unicode-xid"] +wgsl-in = ["hexf-parse", "unicode-xid"] wgsl-out = [] hlsl-out = [] -span = ["codespan-reporting", "termcolor"] validate = [] compact = [] @@ -41,11 +40,11 @@ harness = false arbitrary = { version = "1.3", features = ["derive"], optional = true } bitflags = "2.2" bit-set = "0.5" -termcolor = { version = "1.4.0", optional = true } +termcolor = { version = "1.4.0" } # remove termcolor dep when updating to the next version of codespan-reporting # termcolor minimum version was wrong and was fixed in # https://github.com/brendanzab/codespan/commit/e99c867339a877731437e7ee6a903a3d03b5439e -codespan-reporting = { version = "0.11.0", optional = true } +codespan-reporting = { version = "0.11.0" } rustc-hash = "1.1.0" indexmap = { version = "2", features = ["std"] } log = "0.4" diff --git a/naga/src/arena.rs b/naga/src/arena.rs index 54c92e849a..c37538667f 100644 --- a/naga/src/arena.rs +++ b/naga/src/arena.rs @@ -247,7 +247,6 @@ impl Range { pub struct Arena { /// Values of this arena. data: Vec, - #[cfg(feature = "span")] #[cfg_attr(feature = "serialize", serde(skip))] span_info: Vec, } @@ -269,7 +268,6 @@ impl Arena { pub const fn new() -> Self { Arena { data: Vec::new(), - #[cfg(feature = "span")] span_info: Vec::new(), } } @@ -310,11 +308,8 @@ impl Arena { /// Adds a new value to the arena, returning a typed handle. pub fn append(&mut self, value: T, span: Span) -> Handle { - #[cfg(not(feature = "span"))] - let _ = span; let index = self.data.len(); self.data.push(value); - #[cfg(feature = "span")] self.span_info.push(span); Handle::from_usize(index) } @@ -377,18 +372,10 @@ impl Arena { } pub fn get_span(&self, handle: Handle) -> Span { - #[cfg(feature = "span")] - { - *self - .span_info - .get(handle.index()) - .unwrap_or(&Span::default()) - } - #[cfg(not(feature = "span"))] - { - let _ = handle; - Span::default() - } + *self + .span_info + .get(handle.index()) + .unwrap_or(&Span::default()) } /// Assert that `handle` is valid for this arena. @@ -438,7 +425,6 @@ impl Arena { // Since `predicate` needs mutable access to each element, // we can't feasibly call it twice, so we have to compact // spans by hand in parallel as part of this iteration. - #[cfg(feature = "span")] if keep { self.span_info[retained] = self.span_info[index]; retained += 1; @@ -448,7 +434,6 @@ impl Arena { keep }); - #[cfg(feature = "span")] self.span_info.truncate(retained); } } @@ -463,16 +448,11 @@ where D: serde::Deserializer<'de>, { let data = Vec::deserialize(deserializer)?; - #[cfg(feature = "span")] let span_info = std::iter::repeat(Span::default()) .take(data.len()) .collect(); - Ok(Self { - data, - #[cfg(feature = "span")] - span_info, - }) + Ok(Self { data, span_info }) } } @@ -561,7 +541,6 @@ pub struct UniqueArena { /// promises that its elements "are indexed in a compact range, without /// holes in the range 0..set.len()", so we can always use the indices /// returned by insertion as indices into this vector. - #[cfg(feature = "span")] span_info: Vec, } @@ -570,7 +549,6 @@ impl UniqueArena { pub fn new() -> Self { UniqueArena { set: FastIndexSet::default(), - #[cfg(feature = "span")] span_info: Vec::new(), } } @@ -588,7 +566,6 @@ impl UniqueArena { /// Clears the arena, keeping all allocations. pub fn clear(&mut self) { self.set.clear(); - #[cfg(feature = "span")] self.span_info.clear(); } @@ -596,29 +573,17 @@ impl UniqueArena { /// /// If a value has been inserted multiple times, the span returned is the /// one provided with the first insertion. - /// - /// If the `span` feature is not enabled, always return `Span::default`. - /// This can be detected with [`Span::is_defined`]. pub fn get_span(&self, handle: Handle) -> Span { - #[cfg(feature = "span")] - { - *self - .span_info - .get(handle.index()) - .unwrap_or(&Span::default()) - } - #[cfg(not(feature = "span"))] - { - let _ = handle; - Span::default() - } + *self + .span_info + .get(handle.index()) + .unwrap_or(&Span::default()) } #[cfg(feature = "compact")] pub(crate) fn drain_all(&mut self) -> UniqueArenaDrain { UniqueArenaDrain { inner_elts: self.set.drain(..), - #[cfg(feature = "span")] inner_spans: self.span_info.drain(..), index: Index::new(1).unwrap(), } @@ -628,7 +593,6 @@ impl UniqueArena { #[cfg(feature = "compact")] pub(crate) struct UniqueArenaDrain<'a, T> { inner_elts: indexmap::set::Drain<'a, T>, - #[cfg(feature = "span")] inner_spans: std::vec::Drain<'a, Span>, index: Index, } @@ -642,10 +606,7 @@ impl<'a, T> Iterator for UniqueArenaDrain<'a, T> { Some(elt) => { let handle = Handle::new(self.index); self.index = self.index.checked_add(1).unwrap(); - #[cfg(feature = "span")] let span = self.inner_spans.next().unwrap(); - #[cfg(not(feature = "span"))] - let span = Span::default(); Some((handle, elt, span)) } None => None, @@ -672,27 +633,21 @@ impl UniqueArena { /// If this arena already contains an element that is `Eq` to `value`, /// return a `Handle` to the existing element, and drop `value`. /// - /// When the `span` feature is enabled, if `value` is inserted into the - /// arena, associate `span` with it. An element's span can be retrieved with - /// the [`get_span`] method. + /// If `value` is inserted into the arena, associate `span` with + /// it. An element's span can be retrieved with the [`get_span`] + /// method. /// /// [`Handle`]: Handle /// [`get_span`]: UniqueArena::get_span pub fn insert(&mut self, value: T, span: Span) -> Handle { let (index, added) = self.set.insert_full(value); - #[cfg(feature = "span")] - { - if added { - debug_assert!(index == self.span_info.len()); - self.span_info.push(span); - } - - debug_assert!(self.set.len() == self.span_info.len()); + if added { + debug_assert!(index == self.span_info.len()); + self.span_info.push(span); } - #[cfg(not(feature = "span"))] - let _ = (span, added); + debug_assert!(self.set.len() == self.span_info.len()); Handle::from_usize(index) } @@ -779,14 +734,9 @@ where D: serde::Deserializer<'de>, { let set = FastIndexSet::deserialize(deserializer)?; - #[cfg(feature = "span")] let span_info = std::iter::repeat(Span::default()).take(set.len()).collect(); - Ok(Self { - set, - #[cfg(feature = "span")] - span_info, - }) + Ok(Self { set, span_info }) } } @@ -800,7 +750,6 @@ where let mut arena = Self::default(); for elem in u.arbitrary_iter()? { arena.set.insert(elem?); - #[cfg(feature = "span")] arena.span_info.push(Span::UNDEFINED); } Ok(arena) @@ -810,7 +759,6 @@ where let mut arena = Self::default(); for elem in u.arbitrary_take_rest_iter()? { arena.set.insert(elem?); - #[cfg(feature = "span")] arena.span_info.push(Span::UNDEFINED); } Ok(arena) diff --git a/naga/src/block.rs b/naga/src/block.rs index b375132ef7..0abda9da7c 100644 --- a/naga/src/block.rs +++ b/naga/src/block.rs @@ -8,7 +8,6 @@ use std::ops::{Deref, DerefMut, RangeBounds}; #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct Block { body: Vec, - #[cfg(feature = "span")] #[cfg_attr(feature = "serialize", serde(skip))] span_info: Vec, } @@ -17,27 +16,20 @@ impl Block { pub const fn new() -> Self { Self { body: Vec::new(), - #[cfg(feature = "span")] span_info: Vec::new(), } } pub fn from_vec(body: Vec) -> Self { - #[cfg(feature = "span")] let span_info = std::iter::repeat(Span::default()) .take(body.len()) .collect(); - Self { - body, - #[cfg(feature = "span")] - span_info, - } + Self { body, span_info } } pub fn with_capacity(capacity: usize) -> Self { Self { body: Vec::with_capacity(capacity), - #[cfg(feature = "span")] span_info: Vec::with_capacity(capacity), } } @@ -45,7 +37,6 @@ impl Block { #[allow(unused_variables)] pub fn push(&mut self, end: Statement, span: Span) { self.body.push(end); - #[cfg(feature = "span")] self.span_info.push(span); } @@ -56,43 +47,31 @@ impl Block { } pub fn extend_block(&mut self, other: Self) { - #[cfg(feature = "span")] self.span_info.extend(other.span_info); self.body.extend(other.body); } pub fn append(&mut self, other: &mut Self) { - #[cfg(feature = "span")] self.span_info.append(&mut other.span_info); self.body.append(&mut other.body); } pub fn cull + Clone>(&mut self, range: R) { - #[cfg(feature = "span")] self.span_info.drain(range.clone()); self.body.drain(range); } pub fn splice + Clone>(&mut self, range: R, other: Self) { - #[cfg(feature = "span")] self.span_info.splice(range.clone(), other.span_info); self.body.splice(range, other.body); } pub fn span_iter(&self) -> impl Iterator { - #[cfg(feature = "span")] let span_iter = self.span_info.iter(); - #[cfg(not(feature = "span"))] - let span_iter = std::iter::repeat_with(|| &Span::UNDEFINED); - self.body.iter().zip(span_iter) } pub fn span_iter_mut(&mut self) -> impl Iterator)> { - #[cfg(feature = "span")] let span_iter = self.span_info.iter_mut().map(Some); - #[cfg(not(feature = "span"))] - let span_iter = std::iter::repeat_with(|| None); - self.body.iter_mut().zip(span_iter) } diff --git a/naga/src/proc/emitter.rs b/naga/src/proc/emitter.rs index 281a55e2ad..0df804fff2 100644 --- a/naga/src/proc/emitter.rs +++ b/naga/src/proc/emitter.rs @@ -28,7 +28,6 @@ impl Emitter { #[allow(unused_mut)] let mut span = crate::span::Span::default(); let range = arena.range_from(start_len); - #[cfg(feature = "span")] for handle in range.clone() { span.subsume(arena.get_span(handle)) } diff --git a/naga/src/span.rs b/naga/src/span.rs index 5c617515ff..53246b25d6 100644 --- a/naga/src/span.rs +++ b/naga/src/span.rs @@ -128,7 +128,6 @@ pub type SpanContext = (Span, String); #[derive(Debug, Clone)] pub struct WithSpan { inner: E, - #[cfg(feature = "span")] spans: Vec, } @@ -165,7 +164,6 @@ impl WithSpan { pub const fn new(inner: E) -> Self { Self { inner, - #[cfg(feature = "span")] spans: Vec::new(), } } @@ -182,22 +180,14 @@ impl WithSpan { /// Iterator over stored [`SpanContext`]s. pub fn spans(&self) -> impl ExactSizeIterator { - #[cfg(feature = "span")] - return self.spans.iter(); - #[cfg(not(feature = "span"))] - return std::iter::empty(); + self.spans.iter() } /// Add a new span with description. - #[cfg_attr( - not(feature = "span"), - allow(unused_variables, unused_mut, clippy::missing_const_for_fn) - )] pub fn with_span(mut self, span: Span, description: S) -> Self where S: ToString, { - #[cfg(feature = "span")] if span.is_defined() { self.spans.push((span, description.to_string())); } @@ -223,7 +213,6 @@ impl WithSpan { { WithSpan { inner: self.inner.into(), - #[cfg(feature = "span")] spans: self.spans, } } @@ -234,14 +223,11 @@ impl WithSpan { where F: FnOnce(E) -> WithSpan, { - #[cfg_attr(not(feature = "span"), allow(unused_mut))] let mut res = func(self.inner); - #[cfg(feature = "span")] res.spans.extend(self.spans); res } - #[cfg(feature = "span")] /// Return a [`SourceLocation`] for our first span, if we have one. pub fn location(&self, source: &str) -> Option { if self.spans.is_empty() { @@ -251,14 +237,6 @@ impl WithSpan { Some(self.spans[0].0.location(source)) } - #[cfg(not(feature = "span"))] - #[allow(clippy::missing_const_for_fn)] - /// Return a [`SourceLocation`] for our first span, if we have one. - pub fn location(&self, _source: &str) -> Option { - None - } - - #[cfg(feature = "span")] fn diagnostic(&self) -> codespan_reporting::diagnostic::Diagnostic<()> where E: Error, @@ -286,7 +264,6 @@ impl WithSpan { } /// Emits a summary of the error to standard error stream. - #[cfg(feature = "span")] pub fn emit_to_stderr(&self, source: &str) where E: Error, @@ -295,7 +272,6 @@ impl WithSpan { } /// Emits a summary of the error to standard error stream. - #[cfg(feature = "span")] pub fn emit_to_stderr_with_path(&self, source: &str, path: &str) where E: Error, @@ -311,7 +287,6 @@ impl WithSpan { } /// Emits a summary of the error to a string. - #[cfg(feature = "span")] pub fn emit_to_string(&self, source: &str) -> String where E: Error, @@ -320,7 +295,6 @@ impl WithSpan { } /// Emits a summary of the error to a string. - #[cfg(feature = "span")] pub fn emit_to_string_with_path(&self, source: &str, path: &str) -> String where E: Error, diff --git a/naga/tests/snapshots.rs b/naga/tests/snapshots.rs index ed75805ae0..f50ba99e3c 100644 --- a/naga/tests/snapshots.rs +++ b/naga/tests/snapshots.rs @@ -311,14 +311,10 @@ fn check_targets( #[cfg(all(feature = "deserialize", feature = "spv-out"))] { - let debug_info = if cfg!(feature = "span") { - source_code.map(|code| naga::back::spv::DebugInfo { - source_code: code, - file_name: name.as_ref(), - }) - } else { - None - }; + let debug_info = source_code.map(|code| naga::back::spv::DebugInfo { + source_code: code, + file_name: name.as_ref(), + }); if targets.contains(Targets::SPIRV) { write_output_spv( @@ -791,7 +787,6 @@ fn convert_wgsl() { } } - #[cfg(feature = "span")] { let inputs = [ ("debug-symbol-simple", Targets::SPIRV), diff --git a/naga/tests/wgsl-errors.rs b/naga/tests/wgsl-errors.rs index 6530082776..5ca43a683a 100644 --- a/naga/tests/wgsl-errors.rs +++ b/naga/tests/wgsl-errors.rs @@ -1993,7 +1993,6 @@ fn binding_array_non_struct() { } } -#[cfg(feature = "span")] #[test] fn compaction_preserves_spans() { let source = r#" diff --git a/wgpu-core/Cargo.toml b/wgpu-core/Cargo.toml index 7ac00be230..d7ee9a48ec 100644 --- a/wgpu-core/Cargo.toml +++ b/wgpu-core/Cargo.toml @@ -81,7 +81,7 @@ thiserror = "1" [dependencies.naga] path = "../naga" version = "0.14.0" -features = ["clone", "span", "validate"] +features = ["clone", "validate"] [dependencies.wgt] package = "wgpu-types" From 1512d69f355df63d5766c36e04dfe395e96e4c07 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Fri, 17 Nov 2023 01:57:35 +0100 Subject: [PATCH 2/3] [naga] remove `validate` feature --- naga-cli/Cargo.toml | 1 - naga/Cargo.toml | 1 - naga/benches/criterion.rs | 5 ----- naga/fuzz/Cargo.toml | 2 +- naga/src/valid/analyzer.rs | 4 +--- naga/src/valid/compose.rs | 2 -- naga/src/valid/expression.rs | 5 ----- naga/src/valid/function.rs | 22 +--------------------- naga/src/valid/handles.rs | 10 ---------- naga/src/valid/interface.rs | 26 +------------------------- naga/src/valid/mod.rs | 18 +----------------- wgpu-core/Cargo.toml | 2 +- 12 files changed, 6 insertions(+), 92 deletions(-) diff --git a/naga-cli/Cargo.toml b/naga-cli/Cargo.toml index 0feca747f1..b58170282c 100644 --- a/naga-cli/Cargo.toml +++ b/naga-cli/Cargo.toml @@ -19,7 +19,6 @@ argh = "0.1.5" version = "0.14" path = "../naga" features = [ - "validate", "compact", "wgsl-in", "wgsl-out", diff --git a/naga/Cargo.toml b/naga/Cargo.toml index a1039044f0..aa612725e7 100644 --- a/naga/Cargo.toml +++ b/naga/Cargo.toml @@ -29,7 +29,6 @@ spv-out = ["spirv"] wgsl-in = ["hexf-parse", "unicode-xid"] wgsl-out = [] hlsl-out = [] -validate = [] compact = [] [[bench]] diff --git a/naga/benches/criterion.rs b/naga/benches/criterion.rs index 697467faa6..e57c58a847 100644 --- a/naga/benches/criterion.rs +++ b/naga/benches/criterion.rs @@ -119,7 +119,6 @@ fn gather_modules() -> Vec { fn validation(c: &mut Criterion) { let inputs = gather_modules(); let mut group = c.benchmark_group("valid"); - #[cfg(feature = "validate")] group.bench_function("safe", |b| { let mut validator = naga::valid::Validator::new( naga::valid::ValidationFlags::all(), @@ -131,7 +130,6 @@ fn validation(c: &mut Criterion) { } }); }); - #[cfg(feature = "validate")] group.bench_function("unsafe", |b| { let mut validator = naga::valid::Validator::new( naga::valid::ValidationFlags::empty(), @@ -146,7 +144,6 @@ fn validation(c: &mut Criterion) { } fn backends(c: &mut Criterion) { - #[cfg(feature = "validate")] let inputs = { let mut validator = naga::valid::Validator::new( naga::valid::ValidationFlags::empty(), @@ -158,8 +155,6 @@ fn backends(c: &mut Criterion) { .flat_map(|module| validator.validate(&module).ok().map(|info| (module, info))) .collect::>() }; - #[cfg(not(feature = "validate"))] - let inputs = Vec::<(naga::Module, naga::valid::ModuleInfo)>::new(); let mut group = c.benchmark_group("back"); #[cfg(feature = "wgsl-out")] diff --git a/naga/fuzz/Cargo.toml b/naga/fuzz/Cargo.toml index b7af900847..4285142c06 100644 --- a/naga/fuzz/Cargo.toml +++ b/naga/fuzz/Cargo.toml @@ -16,7 +16,7 @@ libfuzzer-sys = "0.4" [target.'cfg(not(any(target_arch = "wasm32", target_os = "ios")))'.dependencies.naga] path = ".." version = "0.14.0" -features = ["arbitrary", "spv-in", "wgsl-in", "glsl-in", "validate"] +features = ["arbitrary", "spv-in", "wgsl-in", "glsl-in"] [[bin]] name = "spv_parser" diff --git a/naga/src/valid/analyzer.rs b/naga/src/valid/analyzer.rs index b96e482934..df6fc5e9b0 100644 --- a/naga/src/valid/analyzer.rs +++ b/naga/src/valid/analyzer.rs @@ -778,7 +778,6 @@ impl FunctionInfo { let mut requirements = UniformityRequirements::empty(); for expr in range.clone() { let req = self.expressions[expr.index()].uniformity.requirements; - #[cfg(feature = "validate")] if self .flags .contains(super::ValidationFlags::CONTROL_FLOW_UNIFORMITY) @@ -823,7 +822,7 @@ impl FunctionInfo { // The uniformity analysis Naga uses now is less accurate than the one in the WGSL standard, // causing Naga to reject correct uses of `workgroupUniformLoad` in some interesting programs. - /* #[cfg(feature = "validate")] + /* if self .flags .contains(super::ValidationFlags::CONTROL_FLOW_UNIFORMITY) @@ -1060,7 +1059,6 @@ impl ModuleInfo { } #[test] -#[cfg(feature = "validate")] fn uniform_control_flow() { use crate::{Expression as E, Statement as S}; diff --git a/naga/src/valid/compose.rs b/naga/src/valid/compose.rs index b392de57b0..a537e07af5 100644 --- a/naga/src/valid/compose.rs +++ b/naga/src/valid/compose.rs @@ -1,4 +1,3 @@ -#[cfg(feature = "validate")] use crate::proc::TypeResolution; use crate::arena::Handle; @@ -14,7 +13,6 @@ pub enum ComposeError { ComponentType { index: u32 }, } -#[cfg(feature = "validate")] pub fn validate_compose( self_ty_handle: Handle, gctx: crate::proc::GlobalCtx, diff --git a/naga/src/valid/expression.rs b/naga/src/valid/expression.rs index dc70c2e610..840ba90d01 100644 --- a/naga/src/valid/expression.rs +++ b/naga/src/valid/expression.rs @@ -1,9 +1,7 @@ -#[cfg(feature = "validate")] use super::{ compose::validate_compose, validate_atomic_compare_exchange_struct, FunctionInfo, ModuleInfo, ShaderStages, TypeFlags, }; -#[cfg(feature = "validate")] use crate::arena::UniqueArena; use crate::{ @@ -155,14 +153,12 @@ pub enum LiteralError { Width(#[from] super::r#type::WidthError), } -#[cfg(feature = "validate")] struct ExpressionTypeResolver<'a> { root: Handle, types: &'a UniqueArena, info: &'a FunctionInfo, } -#[cfg(feature = "validate")] impl<'a> std::ops::Index> for ExpressionTypeResolver<'a> { type Output = crate::TypeInner; @@ -180,7 +176,6 @@ impl<'a> std::ops::Index> for ExpressionTypeResolver<' } } -#[cfg(feature = "validate")] impl super::Validator { pub(super) fn validate_const_expression( &self, diff --git a/naga/src/valid/function.rs b/naga/src/valid/function.rs index 1264937781..f5da7d0764 100644 --- a/naga/src/valid/function.rs +++ b/naga/src/valid/function.rs @@ -1,8 +1,6 @@ use crate::arena::Handle; -#[cfg(feature = "validate")] use crate::arena::{Arena, UniqueArena}; -#[cfg(feature = "validate")] use super::validate_atomic_compare_exchange_struct; use super::{ @@ -10,10 +8,8 @@ use super::{ ExpressionError, FunctionInfo, ModuleInfo, }; use crate::span::WithSpan; -#[cfg(feature = "validate")] use crate::span::{AddSpan as _, MapErrWithSpan as _}; -#[cfg(feature = "validate")] use bit_set::BitSet; #[derive(Clone, Debug, thiserror::Error)] @@ -174,13 +170,11 @@ bitflags::bitflags! { } } -#[cfg(feature = "validate")] struct BlockInfo { stages: super::ShaderStages, finished: bool, } -#[cfg(feature = "validate")] struct BlockContext<'a> { abilities: ControlFlowAbility, info: &'a FunctionInfo, @@ -194,7 +188,6 @@ struct BlockContext<'a> { return_type: Option>, } -#[cfg(feature = "validate")] impl<'a> BlockContext<'a> { fn new( fun: &'a crate::Function, @@ -263,7 +256,6 @@ impl<'a> BlockContext<'a> { } impl super::Validator { - #[cfg(feature = "validate")] fn validate_call( &mut self, function: Handle, @@ -320,7 +312,6 @@ impl super::Validator { Ok(callee_info.available_stages) } - #[cfg(feature = "validate")] fn emit_expression( &mut self, handle: Handle, @@ -335,7 +326,6 @@ impl super::Validator { } } - #[cfg(feature = "validate")] fn validate_atomic( &mut self, pointer: Handle, @@ -410,7 +400,6 @@ impl super::Validator { Ok(()) } - #[cfg(feature = "validate")] fn validate_block_impl( &mut self, statements: &crate::Block, @@ -920,7 +909,6 @@ impl super::Validator { Ok(BlockInfo { stages, finished }) } - #[cfg(feature = "validate")] fn validate_block( &mut self, statements: &crate::Block, @@ -934,7 +922,6 @@ impl super::Validator { Ok(info) } - #[cfg(feature = "validate")] fn validate_local_var( &self, var: &crate::LocalVariable, @@ -971,16 +958,13 @@ impl super::Validator { fun: &crate::Function, module: &crate::Module, mod_info: &ModuleInfo, - #[cfg_attr(not(feature = "validate"), allow(unused))] entry_point: bool, + entry_point: bool, ) -> Result> { - #[cfg_attr(not(feature = "validate"), allow(unused_mut))] let mut info = mod_info.process_function(fun, module, self.flags, self.capabilities)?; - #[cfg(feature = "validate")] let expression_constness = crate::proc::ExpressionConstnessTracker::from_arena(&fun.expressions); - #[cfg(feature = "validate")] for (var_handle, var) in fun.local_variables.iter() { self.validate_local_var(var, module.to_ctx(), &info, &expression_constness) .map_err(|source| { @@ -994,7 +978,6 @@ impl super::Validator { })?; } - #[cfg(feature = "validate")] for (index, argument) in fun.arguments.iter().enumerate() { match module.types[argument.ty].inner.pointer_space() { Some(crate::AddressSpace::Private | crate::AddressSpace::Function) | None => {} @@ -1027,7 +1010,6 @@ impl super::Validator { } } - #[cfg(feature = "validate")] if let Some(ref result) = fun.result { if !self.types[result.ty.index()] .flags @@ -1049,7 +1031,6 @@ impl super::Validator { if expr.needs_pre_emit() { self.valid_expression_set.insert(handle.index()); } - #[cfg(feature = "validate")] if self.flags.contains(super::ValidationFlags::EXPRESSIONS) { match self.validate_expression(handle, expr, fun, module, &info, mod_info) { Ok(stages) => info.available_stages &= stages, @@ -1061,7 +1042,6 @@ impl super::Validator { } } - #[cfg(feature = "validate")] if self.flags.contains(super::ValidationFlags::BLOCKS) { let stages = self .validate_block( diff --git a/naga/src/valid/handles.rs b/naga/src/valid/handles.rs index ec6dd240c0..e482f293bb 100644 --- a/naga/src/valid/handles.rs +++ b/naga/src/valid/handles.rs @@ -5,16 +5,12 @@ use crate::{ Handle, }; -#[cfg(feature = "validate")] use crate::{Arena, UniqueArena}; -#[cfg(feature = "validate")] use super::ValidationError; -#[cfg(feature = "validate")] use std::{convert::TryInto, hash::Hash, num::NonZeroU32}; -#[cfg(feature = "validate")] impl super::Validator { /// Validates that all handles within `module` are: /// @@ -547,21 +543,18 @@ impl super::Validator { } } -#[cfg(feature = "validate")] impl From for ValidationError { fn from(source: BadHandle) -> Self { Self::InvalidHandle(source.into()) } } -#[cfg(feature = "validate")] impl From for ValidationError { fn from(source: FwdDepError) -> Self { Self::InvalidHandle(source.into()) } } -#[cfg(feature = "validate")] impl From for ValidationError { fn from(source: BadRangeError) -> Self { Self::InvalidHandle(source.into()) @@ -592,7 +585,6 @@ pub struct FwdDepError { depends_on_kind: &'static str, } -#[cfg(feature = "validate")] impl Handle { /// Check that `self` is valid within `arena` using [`Arena::check_contains_handle`]. pub(self) fn check_valid_for(self, arena: &Arena) -> Result<(), InvalidHandleError> { @@ -656,7 +648,6 @@ impl Handle { } } -#[cfg(feature = "validate")] impl crate::arena::Range { pub(self) fn check_valid_for(&self, arena: &Arena) -> Result<(), BadRangeError> { arena.check_contains_range(self) @@ -664,7 +655,6 @@ impl crate::arena::Range { } #[test] -#[cfg(feature = "validate")] fn constant_deps() { use crate::{Constant, Expression, Literal, Span, Type, TypeInner}; diff --git a/naga/src/valid/interface.rs b/naga/src/valid/interface.rs index 2e50f67160..57863048e5 100644 --- a/naga/src/valid/interface.rs +++ b/naga/src/valid/interface.rs @@ -7,7 +7,6 @@ use crate::arena::{Handle, UniqueArena}; use crate::span::{AddSpan as _, MapErrWithSpan as _, SpanProvider as _, WithSpan}; use bit_set::BitSet; -#[cfg(feature = "validate")] const MAX_WORKGROUP_SIZE: u32 = 0x4000; #[derive(Clone, Debug, thiserror::Error)] @@ -110,7 +109,6 @@ pub enum EntryPointError { InvalidLocationsWhileDualSourceBlending { location_mask: BitSet }, } -#[cfg(feature = "validate")] fn storage_usage(access: crate::StorageAccess) -> GlobalUse { let mut storage_usage = GlobalUse::QUERY; if access.contains(crate::StorageAccess::LOAD) { @@ -131,8 +129,6 @@ struct VaryingContext<'a> { location_mask: &'a mut BitSet, built_ins: &'a mut crate::FastHashSet, capabilities: Capabilities, - - #[cfg(feature = "validate")] flags: super::ValidationFlags, } @@ -307,7 +303,6 @@ impl VaryingContext<'_> { self.second_blend_source = true; } else if !self.location_mask.insert(location as usize) { - #[cfg(feature = "validate")] if self.flags.contains(super::ValidationFlags::BINDINGS) { return Err(VaryingError::BindingCollision { location }); } @@ -369,15 +364,12 @@ impl VaryingContext<'_> { let span_context = self.types.get_span_context(ty); match member.binding { None => { - #[cfg(feature = "validate")] if self.flags.contains(super::ValidationFlags::BINDINGS) { return Err(VaryingError::MemberMissingBinding( index as u32, ) .with_span_context(span_context)); } - #[cfg(not(feature = "validate"))] - let _ = index; } Some(ref binding) => self .validate_impl(member.ty, binding) @@ -385,9 +377,7 @@ impl VaryingContext<'_> { } } } - _ => - { - #[cfg(feature = "validate")] + _ => { if self.flags.contains(super::ValidationFlags::BINDINGS) { return Err(VaryingError::MissingBinding.with_span()); } @@ -400,7 +390,6 @@ impl VaryingContext<'_> { } impl super::Validator { - #[cfg(feature = "validate")] pub(super) fn validate_global_var( &self, var: &crate::GlobalVariable, @@ -550,7 +539,6 @@ impl super::Validator { module: &crate::Module, mod_info: &ModuleInfo, ) -> Result> { - #[cfg(feature = "validate")] if ep.early_depth_test.is_some() { let required = Capabilities::EARLY_DEPTH_TEST; if !self.capabilities.contains(required) { @@ -565,7 +553,6 @@ impl super::Validator { } } - #[cfg(feature = "validate")] if ep.stage == crate::ShaderStage::Compute { if ep .workgroup_size @@ -578,12 +565,10 @@ impl super::Validator { return Err(EntryPointError::UnexpectedWorkgroupSize.with_span()); } - #[cfg_attr(not(feature = "validate"), allow(unused_mut))] let mut info = self .validate_function(&ep.function, module, mod_info, true) .map_err(WithSpan::into_other)?; - #[cfg(feature = "validate")] { use super::ShaderStages; @@ -611,8 +596,6 @@ impl super::Validator { location_mask: &mut self.location_mask, built_ins: &mut argument_built_ins, capabilities: self.capabilities, - - #[cfg(feature = "validate")] flags: self.flags, }; ctx.validate(fa.ty, fa.binding.as_ref()) @@ -631,13 +614,10 @@ impl super::Validator { location_mask: &mut self.location_mask, built_ins: &mut result_built_ins, capabilities: self.capabilities, - - #[cfg(feature = "validate")] flags: self.flags, }; ctx.validate(fr.ty, fr.binding.as_ref()) .map_err_inner(|e| EntryPointError::Result(e).with_span())?; - #[cfg(feature = "validate")] if ctx.second_blend_source { // Only the first location may be used whhen dual source blending if ctx.location_mask.len() == 1 && ctx.location_mask.contains(0) { @@ -650,18 +630,15 @@ impl super::Validator { } } - #[cfg(feature = "validate")] if ep.stage == crate::ShaderStage::Vertex && !result_built_ins.contains(&crate::BuiltIn::Position { invariant: false }) { return Err(EntryPointError::MissingVertexOutputPosition.with_span()); } } else if ep.stage == crate::ShaderStage::Vertex { - #[cfg(feature = "validate")] return Err(EntryPointError::MissingVertexOutputPosition.with_span()); } - #[cfg(feature = "validate")] { let used_push_constants = module .global_variables @@ -679,7 +656,6 @@ impl super::Validator { } self.ep_resource_bindings.clear(); - #[cfg(feature = "validate")] for (var_handle, var) in module.global_variables.iter() { let usage = info[var_handle]; if usage.is_empty() { diff --git a/naga/src/valid/mod.rs b/naga/src/valid/mod.rs index 011e90fbc2..70a4d39d2a 100644 --- a/naga/src/valid/mod.rs +++ b/naga/src/valid/mod.rs @@ -45,31 +45,22 @@ bitflags::bitflags! { /// should never panic. /// /// The default value for `ValidationFlags` is - /// `ValidationFlags::all()`. If Naga's `"validate"` feature is - /// enabled, this requests full validation; otherwise, this - /// requests no validation. (The `"validate"` feature is disabled - /// by default.) + /// `ValidationFlags::all()`. #[cfg_attr(feature = "serialize", derive(serde::Serialize))] #[cfg_attr(feature = "deserialize", derive(serde::Deserialize))] #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub struct ValidationFlags: u8 { /// Expressions. - #[cfg(feature = "validate")] const EXPRESSIONS = 0x1; /// Statements and blocks of them. - #[cfg(feature = "validate")] const BLOCKS = 0x2; /// Uniformity of control flow for operations that require it. - #[cfg(feature = "validate")] const CONTROL_FLOW_UNIFORMITY = 0x4; /// Host-shareable structure layouts. - #[cfg(feature = "validate")] const STRUCT_LAYOUTS = 0x8; /// Constants. - #[cfg(feature = "validate")] const CONSTANTS = 0x10; /// Group, binding, and location attributes. - #[cfg(feature = "validate")] const BINDINGS = 0x20; } } @@ -237,7 +228,6 @@ pub enum ValidationError { } impl crate::TypeInner { - #[cfg(feature = "validate")] const fn is_sized(&self) -> bool { match *self { Self::Scalar { .. } @@ -261,7 +251,6 @@ impl crate::TypeInner { } /// Return the `ImageDimension` for which `self` is an appropriate coordinate. - #[cfg(feature = "validate")] const fn image_storage_coordinates(&self) -> Option { match *self { Self::Scalar(crate::Scalar { @@ -316,7 +305,6 @@ impl Validator { self.valid_expression_set.clear(); } - #[cfg(feature = "validate")] fn validate_constant( &self, handle: Handle, @@ -347,7 +335,6 @@ impl Validator { self.reset(); self.reset_types(module.types.len()); - #[cfg(feature = "validate")] Self::validate_module_handles(module).map_err(|e| e.with_span())?; self.layouter.update(module.to_ctx()).map_err(|e| { @@ -397,7 +384,6 @@ impl Validator { } } - #[cfg(feature = "validate")] if self.flags.contains(ValidationFlags::CONSTANTS) { for (handle, _) in module.const_expressions.iter() { self.validate_const_expression(handle, module.to_ctx(), &mod_info) @@ -420,7 +406,6 @@ impl Validator { } } - #[cfg(feature = "validate")] for (var_handle, var) in module.global_variables.iter() { self.validate_global_var(var, module.to_ctx(), &mod_info) .map_err(|source| { @@ -479,7 +464,6 @@ impl Validator { } } -#[cfg(feature = "validate")] fn validate_atomic_compare_exchange_struct( types: &crate::UniqueArena, members: &[crate::StructMember], diff --git a/wgpu-core/Cargo.toml b/wgpu-core/Cargo.toml index d7ee9a48ec..a0c9fdbb19 100644 --- a/wgpu-core/Cargo.toml +++ b/wgpu-core/Cargo.toml @@ -81,7 +81,7 @@ thiserror = "1" [dependencies.naga] path = "../naga" version = "0.14.0" -features = ["clone", "validate"] +features = ["clone"] [dependencies.wgt] package = "wgpu-types" From fc66b8a8134e59ec0170326a4d109b7bbab7f984 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Fri, 17 Nov 2023 02:03:05 +0100 Subject: [PATCH 3/3] add changelog entry --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bdc068661..5b54d6c435 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ Passing an owned value `window` to `Surface` will return a `Surface<'static>`. S #### Naga +- Remove `span` and `validate` features. Always fully validate shader modules, and always track source positions for use in error messages. By @teoxoy in [#4706](https://github.com/gfx-rs/wgpu/pull/4706) - Introduce a new `Scalar` struct type for use in Naga's IR, and update all frontend, middle, and backend code appropriately. By @jimblandy in [#4673](https://github.com/gfx-rs/wgpu/pull/4673). ### Bug Fixes @@ -91,7 +92,7 @@ Passing an owned value `window` to `Surface` will return a `Surface<'static>`. S - Passing a naga module directly to `Device::create_shader_module`. - `InstanceFlags::DEBUG` is enabled. -#### DX12 +#### DX12 - Always use HLSL 2018 when using DXC to compile HLSL shaders. By @daxpedda in [#4629](https://github.com/gfx-rs/wgpu/pull/4629) #### Metal