[clippy] Fix clippy issues after upgrading to 1.47 (#237)

* [clippy] disable match_like_matches_macro for now

* [wgsl-in] Return error instead of panic

* [typifier] Convert panics to return error

Still one panic in clone impl for Resolution,
disabled clippy for that one

* [spv-in] REturn error instead of panic'ing

* [spv-out] Return error instead of panic'ing

* [typifier] Use IncompatibleOperand(s) error

Instead of just Other

* [typifier] Use Expression for op in Error

For IncompatibleOperands

* [typifier] revert last commit
This commit is contained in:
Pelle Johnsen
2020-10-13 23:14:40 +02:00
committed by GitHub
parent b76dff9d9c
commit 6fb195cdc8
6 changed files with 104 additions and 33 deletions

View File

@@ -2,9 +2,16 @@
use super::{Instruction, LogicalLayout, PhysicalLayout, WriterFlags};
use spirv::Word;
use std::collections::hash_map::Entry;
use thiserror::Error;
const BITS_PER_BYTE: crate::Bytes = 8;
#[derive(Clone, Debug, Error)]
pub enum Error {
#[error("can't find local variable: {0:?}")]
UnknownLocalVariable(crate::LocalVariable),
}
struct Block {
label: Option<Instruction>,
body: Vec<Instruction>,
@@ -118,6 +125,9 @@ pub struct Writer {
lookup_global_variable: crate::FastHashMap<crate::Handle<crate::GlobalVariable>, Word>,
}
// type alias, for success return of write_expression
type WriteExpressionOutput = Option<(Word, Option<crate::Handle<crate::Type>>)>;
impl Writer {
pub fn new(header: &crate::Header, writer_flags: WriterFlags) -> Self {
Writer {
@@ -799,7 +809,7 @@ impl Writer {
expression: &crate::Expression,
block: &mut Block,
function: &mut Function,
) -> Option<(Word, Option<crate::Handle<crate::Type>>)> {
) -> Result<WriteExpressionOutput, Error> {
match expression {
crate::Expression::GlobalVariable(handle) => {
let var = &ir_module.global_variables[*handle];
@@ -808,12 +818,12 @@ impl Writer {
&ir_module.global_variables,
*handle,
);
Some((id, Some(var.ty)))
Ok(Some((id, Some(var.ty))))
}
crate::Expression::Constant(handle) => {
let var = &ir_module.constants[*handle];
let id = self.get_constant_id(*handle, ir_module);
Some((id, Some(var.ty)))
Ok(Some((id, Some(var.ty))))
}
crate::Expression::Compose { ty, components } => {
let base_type_id = self.get_type_id(&ir_module.types, LookupType::Handle(*ty));
@@ -822,7 +832,7 @@ impl Writer {
for component in components {
let expression = &ir_function.expressions[*component];
let (component_id, _) = self
.write_expression(ir_module, &ir_function, expression, block, function)
.write_expression(ir_module, &ir_function, expression, block, function)?
.unwrap();
constituent_ids.push(component_id);
}
@@ -868,7 +878,7 @@ impl Writer {
_ => unreachable!(),
};
Some((id, Some(*ty)))
Ok(Some((id, Some(*ty))))
}
crate::Expression::Binary { op, left, right } => {
match op {
@@ -883,7 +893,7 @@ impl Writer {
left_expression,
block,
function,
)
)?
.unwrap();
let (right_id, right_ty) = self
.write_expression(
@@ -892,7 +902,7 @@ impl Writer {
right_expression,
block,
function,
)
)?
.unwrap();
let left_ty = left_ty.unwrap();
@@ -1017,7 +1027,7 @@ impl Writer {
};
block.body.push(instruction);
Some((id, Some(ty)))
Ok(Some((id, Some(ty))))
}
_ => unimplemented!("{:?}", op),
@@ -1025,17 +1035,12 @@ impl Writer {
}
crate::Expression::LocalVariable(variable) => {
let var = &ir_function.local_variables[*variable];
let id = if let Some(local_var) = function
function
.variables
.iter()
.find(|&v| v.name.as_ref().unwrap() == var.name.as_ref().unwrap())
{
local_var.id
} else {
panic!("Could not find: {:?}", var)
};
Some((id, Some(var.ty)))
.map(|local_var| Some((local_var.id, Some(var.ty))))
.ok_or_else(|| Error::UnknownLocalVariable(var.clone()))
}
crate::Expression::FunctionParameter(index) => {
let handle = ir_function.parameter_types.get(*index as usize).unwrap();
@@ -1048,7 +1053,7 @@ impl Writer {
function.parameters[*index as usize].result_id.unwrap(),
None,
));
Some((load_id, Some(*handle)))
Ok(Some((load_id, Some(*handle))))
}
crate::Expression::Call { origin, arguments } => match origin {
crate::FunctionOrigin::Local(local_function) => {
@@ -1059,7 +1064,7 @@ impl Writer {
for argument in arguments {
let expression = &ir_function.expressions[*argument];
let (id, ty) = self
.write_expression(ir_module, ir_function, expression, block, function)
.write_expression(ir_module, ir_function, expression, block, function)?
.unwrap();
// Create variable - OpVariable
@@ -1102,7 +1107,7 @@ impl Writer {
*self.lookup_function.get(local_function).unwrap(),
argument_ids.as_slice(),
));
Some((id, None))
Ok(Some((id, None)))
}
_ => unimplemented!("{:?}", origin),
},
@@ -1112,7 +1117,7 @@ impl Writer {
convert,
} => {
if !convert {
return None;
return Ok(None);
}
let (expr_id, expr_type) = self
@@ -1122,7 +1127,7 @@ impl Writer {
&ir_function.expressions[*expr],
block,
function,
)
)?
.unwrap();
let id = self.generate_id();
@@ -1177,7 +1182,7 @@ impl Writer {
block.body.push(instruction);
Some((id, None))
Ok(Some((id, None)))
}
_ => unimplemented!("{:?}", expression),
}
@@ -1214,6 +1219,7 @@ impl Writer {
&mut block,
function,
)
.unwrap()
.unwrap();
let id = match *expression {
@@ -1251,6 +1257,7 @@ impl Writer {
&mut block,
function,
)
.unwrap()
.unwrap();
let (value_id, value_ty) = self
.write_expression(
@@ -1260,6 +1267,7 @@ impl Writer {
&mut block,
function,
)
.unwrap()
.unwrap();
let value_id = match value_expression {

View File

@@ -51,4 +51,5 @@ pub enum Error {
BadString,
IncompleteData,
InvalidTerminator,
UnexpectedComparisonType(Handle<crate::Type>),
}

View File

@@ -1477,7 +1477,9 @@ impl<I: Iterator<Item = u32>> Parser<I> {
};
*comparison = true;
}
_ => panic!("Unexpected comparison type {:?}", ty),
_ => {
return Err(Error::UnexpectedComparisonType(handle));
}
}
}

View File

@@ -1787,7 +1787,13 @@ impl Parser {
}
Ok(true) => {}
Ok(false) => {
assert_eq!(self.scopes, Vec::new());
if !self.scopes.is_empty() {
return Err(ParseError {
error: Error::Other,
scopes: std::mem::replace(&mut self.scopes, Vec::new()),
pos: (0, 0),
});
};
return Ok(module);
}
}

View File

@@ -4,7 +4,11 @@
//!
//! To improve performance and reduce memory usage, most structures are stored
//! in an [`Arena`], and can be retrieved using the corresponding [`Handle`].
#![allow(clippy::new_without_default, clippy::unneeded_field_pattern)]
#![allow(
clippy::new_without_default,
clippy::unneeded_field_pattern,
clippy::match_like_matches_macro
)]
#![deny(clippy::panic)]
mod arena;

View File

@@ -29,6 +29,7 @@ impl Clone for Resolution {
columns,
width,
},
#[allow(clippy::panic)]
_ => panic!("Unepxected clone type: {:?}", v),
}),
}
@@ -50,6 +51,14 @@ pub enum ResolveError {
FunctionReturnsVoid,
#[error("Type is not found in the given immutable arena")]
TypeNotFound,
#[error("Incompatible operand: {op} {operand}")]
IncompatibleOperand { op: String, operand: String },
#[error("Incompatible operands: {left} {op} {right}")]
IncompatibleOperands {
op: String,
left: String,
right: String,
},
}
pub struct ResolveContext<'a> {
@@ -105,7 +114,12 @@ impl Typifier {
kind: crate::ScalarKind::Float,
width,
}),
ref other => panic!("Can't access into {:?}", other),
ref other => {
return Err(ResolveError::IncompatibleOperand {
op: "access".to_string(),
operand: format!("{:?}", other),
})
}
},
crate::Expression::AccessIndex { base, index } => match *self.get(base, types) {
crate::TypeInner::Vector { size, kind, width } => {
@@ -135,7 +149,12 @@ impl Typifier {
.ok_or(ResolveError::InvalidAccessIndex)?;
Resolution::Handle(member.ty)
}
ref other => panic!("Can't access into {:?}", other),
ref other => {
return Err(ResolveError::IncompatibleOperand {
op: "access index".to_string(),
operand: format!("{:?}", other),
})
}
},
crate::Expression::Constant(h) => Resolution::Handle(ctx.constants[h].ty),
crate::Expression::Compose { ty, .. } => Resolution::Handle(ty),
@@ -192,7 +211,13 @@ impl Typifier {
kind: crate::ScalarKind::Float,
width,
}),
_ => panic!("Incompatible arguments {:?} x {:?}", ty_left, ty_right),
_ => {
return Err(ResolveError::IncompatibleOperands {
op: "x".to_string(),
left: format!("{:?}", ty_left),
right: format!("{:?}", ty_right),
})
}
}
}
}
@@ -224,7 +249,12 @@ impl Typifier {
rows: columns,
width,
}),
ref other => panic!("incompatible transpose of {:?}", other),
ref other => {
return Err(ResolveError::IncompatibleOperand {
op: "transpose".to_string(),
operand: format!("{:?}", other),
})
}
},
crate::Expression::DotProduct(left_expr, _) => match *self.get(left_expr, types) {
crate::TypeInner::Vector {
@@ -232,7 +262,12 @@ impl Typifier {
size: _,
width,
} => Resolution::Value(crate::TypeInner::Scalar { kind, width }),
ref other => panic!("incompatible dot of {:?}", other),
ref other => {
return Err(ResolveError::IncompatibleOperand {
op: "dot product".to_string(),
operand: format!("{:?}", other),
})
}
},
crate::Expression::CrossProduct(_, _) => unimplemented!(),
crate::Expression::As {
@@ -248,7 +283,12 @@ impl Typifier {
size,
width,
} => Resolution::Value(crate::TypeInner::Vector { kind, size, width }),
ref other => panic!("incompatible as of {:?}", other),
ref other => {
return Err(ResolveError::IncompatibleOperand {
op: "as".to_string(),
operand: format!("{:?}", other),
})
}
},
crate::Expression::Derivative { .. } => unimplemented!(),
crate::Expression::Call {
@@ -260,13 +300,23 @@ impl Typifier {
| crate::TypeInner::Scalar { kind, width } => {
Resolution::Value(crate::TypeInner::Scalar { kind, width })
}
ref other => panic!("Unexpected argument {:?} on {}", other, name),
ref other => {
return Err(ResolveError::IncompatibleOperand {
op: name.clone(),
operand: format!("{:?}", other),
})
}
},
"dot" => match *self.get(arguments[0], types) {
crate::TypeInner::Vector { kind, width, .. } => {
Resolution::Value(crate::TypeInner::Scalar { kind, width })
}
ref other => panic!("Unexpected argument {:?} on {}", other, name),
ref other => {
return Err(ResolveError::IncompatibleOperand {
op: name.clone(),
operand: format!("{:?}", other),
})
}
},
//Note: `cross` is here too, we still need to figure out what to do with it
"abs" | "atan2" | "cos" | "sin" | "floor" | "inverse" | "normalize" | "min"