-
Notifications
You must be signed in to change notification settings - Fork 0
Description
The problem
There's a pattern which we use in a lot of places in codebase where:
- Somewhere checks an
Expressionfor what type ofExpressionit is (e.g. is it one of the literal types e.g.BooleanLiteral). - If so, it calls another function passing that
Expression. - The callee contains a
matchwhich again checks the type of theExpression, with an_ => unreachable!()arm for the types which aren't literals.
This pattern is extremely common in transformer e.g.:
impl<'a> ClassProperties<'a, '_> {
fn enter_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
match expr {
Expression::PrivateFieldExpression(_) => {
self.transform_private_field_expression(expr, ctx);
}
// ...
}
}
fn transform_private_field_expression(
&mut self,
expr: &mut Expression<'a>,
ctx: &mut TraverseCtx<'a>,
) {
let Expression::PrivateFieldExpression(field_expr) = expr else { unreachable!() };
// ...
}
}Code for caller Code for callee
@camc314 Has also pointed out that it's also a common pattern in linter.
Why is it bad?
It's both unperformant (unnecessary checks) and error-prone - we learn about mistakes when there's a "entered unreachable code" panic, rather than getting a compile-time error because types don't match.
Possible solution
It'd be helpful if we had some form of "enum narrowing" where you can convert an enum into either:
- A type which represents only a single variant.
e.g. convert a &mut Expression into a &mut ExpressionWhichIsPrivateField.
transform_private_field_expression in example above would take a &mut ExpressionWhichIsPrivateField, which it can unwrap into a &mut PrivateFieldExpression, or expand it back to a &mut Expression in order to replace it with another type of Expression. Both of these would be zero-cost and infallible.
- A type which has a narrower set of variants.
e.g. convert an &Expression into a &ExpressionWhichIsLiteral.
let expr: &Expression = get_expression_somehow();
if let Some(expr_literal) = expr.as_literal() {
// `expr_literal` is an `&ExpressionLiteral` enum which only has variants
// for `BooleanLiteral`, `StringLiteral` etc.
// Pass `expr_literal` to the next function, instead of passing `expr`.
}Ditto for converting AstKind into e.g. MemberExpressionKind which can only be one of the 3 member expression types.
We already have this ability to a degree via the "enum inheritance" in AST e.g. expr.as_member_expression(). But it'd be useful to expand it to support the above use cases.
For AST types and AstKind we have complete control over the memory layouts of these types, so could codegen these conversions and make them very cheap (just a check and zero-cost transmute).