Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce size of parse error #6146

Merged
merged 1 commit into from
Aug 27, 2024
Merged

Reduce size of parse error #6146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 89 additions & 62 deletions naga/src/front/wgsl/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ use std::ops::Range;
use termcolor::{ColorChoice, NoColor, StandardStream};
use thiserror::Error;

#[cfg(test)]
use std::mem::size_of;

#[derive(Clone, Debug)]
pub struct ParseError {
message: String,
Expand Down Expand Up @@ -144,7 +147,7 @@ pub enum InvalidAssignmentType {
}

#[derive(Clone, Debug)]
pub enum Error<'a> {
pub(crate) enum Error<'a> {
Unexpected(Span, ExpectedToken<'a>),
UnexpectedComponents(Span),
UnexpectedOperationInConstContext(Span),
Expand All @@ -154,8 +157,8 @@ pub enum Error<'a> {
BadTexture(Span),
BadTypeCast {
span: Span,
from_type: String,
to_type: String,
from_type: Box<str>,
to_type: Box<str>,
},
BadTextureSampleType {
span: Span,
Expand Down Expand Up @@ -188,8 +191,8 @@ pub enum Error<'a> {
TypeNotInferable(Span),
InitializationTypeMismatch {
name: Span,
expected: String,
got: String,
expected: Box<str>,
got: Box<str>,
},
DeclMissingTypeAndInit(Span),
MissingAttribute(&'static str, Span),
Expand Down Expand Up @@ -232,7 +235,7 @@ pub enum Error<'a> {
/// name is `decl` has an identifier at `reference` whose definition is
/// the next declaration in the cycle. The last pair's `reference` is
/// the same identifier as `ident`, above.
path: Vec<(Span, Span)>,
path: Box<[(Span, Span)]>,
},
InvalidSwitchValue {
uint: bool,
Expand All @@ -251,32 +254,41 @@ pub enum Error<'a> {
ExpectedNonNegative(Span),
ExpectedPositiveArrayLength(Span),
MissingWorkgroupSize(Span),
ConstantEvaluatorError(ConstantEvaluatorError, Span),
AutoConversion {
dest_span: Span,
dest_type: String,
source_span: Span,
source_type: String,
},
AutoConversionLeafScalar {
dest_span: Span,
dest_scalar: String,
source_span: Span,
source_type: String,
},
ConcretizationFailed {
expr_span: Span,
expr_type: String,
scalar: String,
inner: ConstantEvaluatorError,
},
ConstantEvaluatorError(Box<ConstantEvaluatorError>, Span),
AutoConversion(Box<AutoConversionError>),
AutoConversionLeafScalar(Box<AutoConversionLeafScalarError>),
ConcretizationFailed(Box<ConcretizationFailedError>),
ExceededLimitForNestedBraces {
span: Span,
limit: u8,
},
PipelineConstantIDValue(Span),
}

#[derive(Clone, Debug)]
pub(crate) struct AutoConversionError {
pub dest_span: Span,
pub dest_type: Box<str>,
pub source_span: Span,
pub source_type: Box<str>,
}

#[derive(Clone, Debug)]
pub(crate) struct AutoConversionLeafScalarError {
pub dest_span: Span,
pub dest_scalar: Box<str>,
pub source_span: Span,
pub source_type: Box<str>,
}

#[derive(Clone, Debug)]
pub(crate) struct ConcretizationFailedError {
pub expr_span: Span,
pub expr_type: Box<str>,
pub scalar: Box<str>,
pub inner: ConstantEvaluatorError,
}

impl<'a> Error<'a> {
#[cold]
#[inline(never)]
Expand Down Expand Up @@ -738,45 +750,55 @@ impl<'a> Error<'a> {
)],
notes: vec![],
},
Error::AutoConversion { dest_span, ref dest_type, source_span, ref source_type } => ParseError {
message: format!("automatic conversions cannot convert `{source_type}` to `{dest_type}`"),
labels: vec![
(
dest_span,
format!("a value of type {dest_type} is required here").into(),
),
(
source_span,
format!("this expression has type {source_type}").into(),
)
],
notes: vec![],
Error::AutoConversion(ref error) => {
// destructuring ensures all fields are handled
let AutoConversionError { dest_span, ref dest_type, source_span, ref source_type } = **error;
ParseError {
message: format!("automatic conversions cannot convert `{source_type}` to `{dest_type}`"),
labels: vec![
(
dest_span,
format!("a value of type {dest_type} is required here").into(),
),
(
source_span,
format!("this expression has type {source_type}").into(),
)
],
notes: vec![],
}
},
Error::AutoConversionLeafScalar { dest_span, ref dest_scalar, source_span, ref source_type } => ParseError {
message: format!("automatic conversions cannot convert elements of `{source_type}` to `{dest_scalar}`"),
labels: vec![
(
dest_span,
format!("a value with elements of type {dest_scalar} is required here").into(),
),
(
source_span,
format!("this expression has type {source_type}").into(),
)
],
notes: vec![],
Error::AutoConversionLeafScalar(ref error) => {
let AutoConversionLeafScalarError { dest_span, ref dest_scalar, source_span, ref source_type } = **error;
ParseError {
message: format!("automatic conversions cannot convert elements of `{source_type}` to `{dest_scalar}`"),
labels: vec![
(
dest_span,
format!("a value with elements of type {dest_scalar} is required here").into(),
),
(
source_span,
format!("this expression has type {source_type}").into(),
)
],
notes: vec![],
}
},
Error::ConcretizationFailed { expr_span, ref expr_type, ref scalar, ref inner } => ParseError {
message: format!("failed to convert expression to a concrete type: {}", inner),
labels: vec![
(
expr_span,
format!("this expression has type {}", expr_type).into(),
)
],
notes: vec![
format!("the expression should have been converted to have {} scalar type", scalar),
]
Error::ConcretizationFailed(ref error) => {
let ConcretizationFailedError { expr_span, ref expr_type, ref scalar, ref inner } = **error;
ParseError {
message: format!("failed to convert expression to a concrete type: {}", inner),
labels: vec![
(
expr_span,
format!("this expression has type {}", expr_type).into(),
)
],
notes: vec![
format!("the expression should have been converted to have {} scalar type", scalar),
]
}
},
Error::ExceededLimitForNestedBraces { span, limit } => ParseError {
message: "brace nesting limit reached".into(),
Expand All @@ -796,3 +818,8 @@ impl<'a> Error<'a> {
}
}
}

#[test]
fn test_error_size() {
assert!(size_of::<Error<'_>>() <= 48);
}
4 changes: 2 additions & 2 deletions naga/src/front/wgsl/lower/construction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -530,11 +530,11 @@ impl<'source, 'temp> Lowerer<'source, 'temp> {

// Bad conversion (type cast)
(Components::One { span, ty_inner, .. }, constructor) => {
let from_type = ty_inner.to_wgsl(&ctx.module.to_ctx());
let from_type = ty_inner.to_wgsl(&ctx.module.to_ctx()).into();
return Err(Error::BadTypeCast {
span,
from_type,
to_type: constructor.to_error_string(ctx),
to_type: constructor.to_error_string(ctx).into(),
});
}

Expand Down
41 changes: 23 additions & 18 deletions naga/src/front/wgsl/lower/conversion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
//! WGSL's automatic conversions for abstract types.

use crate::front::wgsl::error::{
AutoConversionError, AutoConversionLeafScalarError, ConcretizationFailedError,
};
use crate::{Handle, Span};

impl<'source, 'temp, 'out> super::ExpressionContext<'source, 'temp, 'out> {
Expand Down Expand Up @@ -39,15 +42,17 @@ impl<'source, 'temp, 'out> super::ExpressionContext<'source, 'temp, 'out> {
Some(scalars) => scalars,
None => {
let gctx = &self.module.to_ctx();
let source_type = expr_resolution.to_wgsl(gctx);
let dest_type = goal_ty.to_wgsl(gctx);

return Err(super::Error::AutoConversion {
dest_span: goal_span,
dest_type,
source_span: expr_span,
source_type,
});
let source_type = expr_resolution.to_wgsl(gctx).into();
let dest_type = goal_ty.to_wgsl(gctx).into();

return Err(super::Error::AutoConversion(Box::new(
AutoConversionError {
dest_span: goal_span,
dest_type,
source_span: expr_span,
source_type,
},
)));
}
};

Expand Down Expand Up @@ -79,13 +84,13 @@ impl<'source, 'temp, 'out> super::ExpressionContext<'source, 'temp, 'out> {

let make_error = || {
let gctx = &self.module.to_ctx();
let source_type = expr_resolution.to_wgsl(gctx);
super::Error::AutoConversionLeafScalar {
let source_type = expr_resolution.to_wgsl(gctx).into();
super::Error::AutoConversionLeafScalar(Box::new(AutoConversionLeafScalarError {
dest_span: goal_span,
dest_scalar: goal_scalar.to_wgsl(),
dest_scalar: goal_scalar.to_wgsl().into(),
source_span: expr_span,
source_type,
}
}))
};

let expr_scalar = match expr_inner.scalar() {
Expand Down Expand Up @@ -116,7 +121,7 @@ impl<'source, 'temp, 'out> super::ExpressionContext<'source, 'temp, 'out> {
if let crate::TypeInner::Array { .. } = *expr_inner {
self.as_const_evaluator()
.cast_array(expr, goal_scalar, expr_span)
.map_err(|err| super::Error::ConstantEvaluatorError(err, expr_span))
.map_err(|err| super::Error::ConstantEvaluatorError(err.into(), expr_span))
} else {
let cast = crate::Expression::As {
expr,
Expand Down Expand Up @@ -254,12 +259,12 @@ impl<'source, 'temp, 'out> super::ExpressionContext<'source, 'temp, 'out> {
// it has one. Also, avoid holding the borrow of `inner`
// across the call to `cast_array`.
let expr_type = &self.typifier()[expr];
super::Error::ConcretizationFailed {
super::Error::ConcretizationFailed(Box::new(ConcretizationFailedError {
expr_span,
expr_type: expr_type.to_wgsl(&self.module.to_ctx()),
scalar: concretized.to_wgsl(),
expr_type: expr_type.to_wgsl(&self.module.to_ctx()).into(),
scalar: concretized.to_wgsl().into(),
inner: err,
}
}))
})?;
}
}
Expand Down
Loading
Loading