T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

ISLE: pattern type is always known (#6144)

Browse Source 
While type-checking the AST for a pattern, ISLE was passing in an
`Option<TypeId>` for the expected result type of the pattern. However,
at every call we either passed `Some` type explicitly, or passed the
parent's expected type in a self-recursive call.

Therefore, by induction, `expected_ty` is never `None`. So this PR
unwraps the type everywhere. That in turn shows that a bunch of error
messages were unreachable, so this deletes a bunch of error-handling
code.

In addition, this function returned the type it computed for the
sub-pattern, but that information is already available in the
sub-pattern itself. Not only that but the type should always be equal to
`expected_ty`; when it isn't, we've reported a type error and are just
trying to check for more errors.

Most callers ignored the returned type but in some cases we used it to
try to avoid emitting useless error messages. I've preserved that
behavior for bind-patterns.

For and-patterns, the returned type looked like it was being used, but
because `expected_ty` was never `None`, the fallback of "fill in with
the sub-pattern's type" never fired. So I've deleted that fallback.

Finally, this reverts #4915 (9d99eff6f9)
which was introduced to flatten nested and-patterns, to simplify overlap
checking. However, the visitor trait used by trie_again effectively
flattens and-patterns anyway, so the current representation used for
overlap checking doesn't need this any more.
This commit is contained in:
Jamey Sharp
2023-04-05 09:22:31 -07:00
committed by GitHub
parent d45cbba83f
commit 34c282ac2e
1 changed files with 75 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files

188
cranelift/isle/isle/src/sema.rs
View File

@@ -1852,32 +1852,26 @@ impl TermEnv {
&self, &self,
tyenv: &mut TypeEnv, tyenv: &mut TypeEnv,
pat: &ast::Pattern, pat: &ast::Pattern,
expected_ty: Option<TypeId>, expected_ty: TypeId,
bindings: &mut Bindings, bindings: &mut Bindings,
) -> Option<(Pattern, TypeId)> { ) -> Option<Pattern> {
log!("translate_pattern: {:?}", pat); log!("translate_pattern: {:?}", pat);
log!("translate_pattern: bindings = {:?}", bindings); log!("translate_pattern: bindings = {:?}", bindings);
match pat { match pat {
// TODO: flag on primitive type decl indicating it's an integer type? // TODO: flag on primitive type decl indicating it's an integer type?
&ast::Pattern::ConstInt { val, pos } => { &ast::Pattern::ConstInt { val, pos } => {
let ty = match expected_ty { let ty = &tyenv.types[expected_ty.index()];
Some(t) => t, if !ty.is_prim() {
None => {
tyenv.report_error(pos, "Need an implied type for an integer constant");
return None;
}
};
if !tyenv.types[ty.index()].is_prim() {
tyenv.report_error( tyenv.report_error(
pos, pos,
format!( format!(
"expected non-primitive type {}, but found integer literal '{}'", "expected non-primitive type {}, but found integer literal '{}'",
tyenv.types[ty.index()].name(tyenv), ty.name(tyenv),
val, val,
), ),
); );
} }
Some((Pattern::ConstInt(ty, val), ty)) Some(Pattern::ConstInt(expected_ty, val))
} }
&ast::Pattern::ConstPrim { ref val, pos } => { &ast::Pattern::ConstPrim { ref val, pos } => {
let val = tyenv.intern_mut(val); let val = tyenv.intern_mut(val);
@@ -1888,53 +1882,36 @@ impl TermEnv {
return None; return None;
} }
}; };
if expected_ty.is_some() && expected_ty != Some(const_ty) { if expected_ty != const_ty {
tyenv.report_error(pos, "Type mismatch for constant"); tyenv.report_error(pos, "Type mismatch for constant");
} }
Some((Pattern::ConstPrim(const_ty, val), const_ty)) Some(Pattern::ConstPrim(const_ty, val))
} }
&ast::Pattern::Wildcard { pos } => { &ast::Pattern::Wildcard { .. } => Some(Pattern::Wildcard(expected_ty)),
let ty = match expected_ty { &ast::Pattern::And { ref subpats, .. } => {
Some(t) => t, // If any of the subpatterns fails to type-check, we'll report
None => { // an error at that point. Here, just skip it and keep looking
tyenv.report_error(pos, "Need an implied type for a wildcard"); // for more errors.
return None; let children = subpats
} .iter()
}; .filter_map(|subpat| {
Some((Pattern::Wildcard(ty), ty)) self.translate_pattern(tyenv, subpat, expected_ty, bindings)
} })
&ast::Pattern::And { ref subpats, pos } => { .collect();
let mut expected_ty = expected_ty; Some(Pattern::And(expected_ty, children))
let mut children = vec![];
for subpat in subpats {
let (subpat, ty) = unwrap_or_continue!(self.translate_pattern(
tyenv,
subpat,
expected_ty,
bindings,
));
expected_ty = expected_ty.or(Some(ty));
// Normalize nested `And` nodes to a single vector of conjuncts.
match subpat {
Pattern::And(_, subpat_children) => children.extend(subpat_children),
_ => children.push(subpat),
}
}
if expected_ty.is_none() {
tyenv.report_error(pos, "No type for (and ...) form.".to_string());
return None;
}
let ty = expected_ty.unwrap();
Some((Pattern::And(ty, children), ty))
} }
&ast::Pattern::BindPattern { &ast::Pattern::BindPattern {
ref var, ref var,
ref subpat, ref subpat,
pos, pos,
} => { } => {
// Do the subpattern first so we can resolve the type for sure. let subpat = self.translate_pattern(tyenv, subpat, expected_ty, bindings)?;
let (subpat, ty) = self.translate_pattern(tyenv, subpat, expected_ty, bindings)?;
// The sub-pattern's type should be `expected_ty`. If it isn't,
// we've already reported a type error about it, but continue
// using the type we actually found in hopes that we'll
// generate fewer follow-on error messages.
let ty = subpat.ty();
let name = tyenv.intern_mut(var); let name = tyenv.intern_mut(var);
if bindings.lookup(name).is_some() { if bindings.lookup(name).is_some() {
@@ -1945,7 +1922,7 @@ impl TermEnv {
// Try to keep going. // Try to keep going.
} }
let id = bindings.add_var(name, ty); let id = bindings.add_var(name, ty);
Some((Pattern::BindPattern(ty, id, Box::new(subpat)), ty)) Some(Pattern::BindPattern(ty, id, Box::new(subpat)))
} }
&ast::Pattern::Var { ref var, pos } => { &ast::Pattern::Var { ref var, pos } => {
// Look up the variable; if it has already been bound, // Look up the variable; if it has already been bound,
@@ -1956,38 +1933,27 @@ impl TermEnv {
let name = tyenv.intern_mut(var); let name = tyenv.intern_mut(var);
match bindings.lookup(name) { match bindings.lookup(name) {
None => { None => {
let ty = match expected_ty { let id = bindings.add_var(name, expected_ty);
Some(ty) => ty, Some(Pattern::BindPattern(
None => { expected_ty,
tyenv.report_error( id,
pos, Box::new(Pattern::Wildcard(expected_ty)),
format!("Variable pattern '{}' not allowed in context without explicit type", var.0),
);
return None;
}
};
let id = bindings.add_var(name, ty);
Some((
Pattern::BindPattern(ty, id, Box::new(Pattern::Wildcard(ty))),
ty,
)) ))
} }
Some(bv) => { Some(bv) => {
let ty = match expected_ty { if expected_ty != bv.ty {
None => bv.ty, tyenv.report_error(
Some(expected_ty) if expected_ty == bv.ty => bv.ty, pos,
Some(expected_ty) => { format!(
tyenv.report_error( "Mismatched types: pattern expects type '{}' but already-bound var '{}' has type '{}'",
pos, tyenv.types[expected_ty.index()].name(tyenv),
format!( var.0,
"Mismatched types: pattern expects type '{}' but already-bound var '{}' has type '{}'", tyenv.types[bv.ty.index()].name(tyenv),
tyenv.types[expected_ty.index()].name(tyenv), ),
var.0, );
tyenv.types[bv.ty.index()].name(tyenv))); // Try to keep going for more errors.
bv.ty // Try to keep going for more errors. }
} Some(Pattern::Var(bv.ty, bv.id))
};
Some((Pattern::Var(ty, bv.id), ty))
} }
} }
} }
@@ -2012,35 +1978,33 @@ impl TermEnv {
// return type of the term. Insert an implicit // return type of the term. Insert an implicit
// converter if needed. // converter if needed.
let ret_ty = termdata.ret_ty; let ret_ty = termdata.ret_ty;
let ty = match expected_ty { if expected_ty != ret_ty {
None => ret_ty, // Can we do an implicit type conversion? Look
Some(expected_ty) if expected_ty == ret_ty => ret_ty, // up the converter term, if any. If one has
Some(expected_ty) => { // been registered, and the term has an
// Can we do an implicit type conversion? Look // extractor, then build an expanded AST node
// up the converter term, if any. If one has // right here and recurse on it.
// been registered, and the term has an if let Some(expanded_pattern) =
// extractor, then build an expanded AST node self.maybe_implicit_convert_pattern(tyenv, pat, ret_ty, expected_ty)
// right here and recurse on it. {
if let Some(expanded_pattern) = return self.translate_pattern(
self.maybe_implicit_convert_pattern(tyenv, pat, ret_ty, expected_ty) tyenv,
{ &expanded_pattern,
return self.translate_pattern( expected_ty,
tyenv, bindings,
&expanded_pattern, );
Some(expected_ty),
bindings,
);
}
tyenv.report_error(
pos,
format!(
"Mismatched types: pattern expects type '{}' but term has return type '{}'",
tyenv.types[expected_ty.index()].name(tyenv),
tyenv.types[ret_ty.index()].name(tyenv)));
ret_ty // Try to keep going for more errors.
} }
};
tyenv.report_error(
pos,
format!(
"Mismatched types: pattern expects type '{}' but term has return type '{}'",
tyenv.types[expected_ty.index()].name(tyenv),
tyenv.types[ret_ty.index()].name(tyenv),
),
);
// Try to keep going for more errors.
}
termdata.check_args_count(args, tyenv, pos, sym); termdata.check_args_count(args, tyenv, pos, sym);
@@ -2080,7 +2044,7 @@ impl TermEnv {
} }
let subpats = self.translate_args(args, termdata, tyenv, bindings); let subpats = self.translate_args(args, termdata, tyenv, bindings);
Some((Pattern::Term(ty, tid, subpats), ty)) Some(Pattern::Term(ret_ty, tid, subpats))
} }
&ast::Pattern::MacroArg { .. } => unreachable!(), &ast::Pattern::MacroArg { .. } => unreachable!(),
} }
@@ -2095,8 +2059,7 @@ impl TermEnv {
) -> Vec<Pattern> { ) -> Vec<Pattern> {
args.iter() args.iter()
.zip(termdata.arg_tys.iter()) .zip(termdata.arg_tys.iter())
.filter_map(|(arg, &arg_ty)| self.translate_pattern(tyenv, arg, Some(arg_ty), bindings)) .filter_map(|(arg, &arg_ty)| self.translate_pattern(tyenv, arg, arg_ty, bindings))
.map(|(subpat, _)| subpat)
.collect() .collect()
} }
@@ -2413,8 +2376,7 @@ impl TermEnv {
root_flags, root_flags,
/* on_lhs */ true, /* on_lhs */ true,
)?; )?;
let ty = rhs.ty(); let lhs = self.translate_pattern(tyenv, &iflet.pattern, rhs.ty(), bindings)?;
let (lhs, _lhs_ty) = self.translate_pattern(tyenv, &iflet.pattern, Some(ty), bindings)?;
Some(IfLet { lhs, rhs }) Some(IfLet { lhs, rhs })
} }