Skip to content

Commit

Permalink
[move][move-2024][enums] Fix a small issue when match is malformed (M…
Browse files Browse the repository at this point in the history
…ystenLabs#18387)

## Description 

This fixes a bug that was previously a compiler crash when a match was
malformed and counterexample generation occurred. This was due to some
changes for parser resilience combined with moving match analysis to
typing.

## Test plan 

Added the repro test, and it now produces the error as expected.

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
  • Loading branch information
cgswords authored Jun 26, 2024
1 parent 8fafac2 commit 32d0433
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -88,16 +88,16 @@ pub(super) fn compile_match(
subject: T::Exp,
arms: Spanned<Vec<T::MatchArm>>,
) -> T::Exp {
let arms_loc = arms.loc;
let loc = arms.loc;
// NB: `from` also flattens `or` and converts constants into guards.
let (pattern_matrix, arms) = PatternMatrix::from(context, subject.ty.clone(), arms.value);
let (pattern_matrix, arms) = PatternMatrix::from(context, loc, subject.ty.clone(), arms.value);

let mut compilation_results: BTreeMap<usize, WorkResult> = BTreeMap::new();

let (mut initial_binders, init_subject, match_subject) = {
let subject_var = context.new_match_var("unpack_subject".to_string(), arms_loc);
let subject_var = context.new_match_var("unpack_subject".to_string(), loc);
let subject_loc = subject.exp.loc;
let match_var = context.new_match_var("match_subject".to_string(), arms_loc);
let match_var = context.new_match_var("match_subject".to_string(), loc);

let subject_entry = FringeEntry {
var: subject_var,
Expand All @@ -122,7 +122,7 @@ pub(super) fn compile_match(
};

let subject_borrow = {
let lhs_loc = arms_loc;
let lhs_loc = loc;
let lhs_lvalue = make_lvalue(match_var, Mutability::Imm, subject_borrow_rhs.ty.clone());
let binder = T::SequenceItem_::Bind(
sp(lhs_loc, vec![lhs_lvalue]),
Expand Down Expand Up @@ -241,7 +241,7 @@ pub(super) fn compile_match(
ice_assert!(
context.env,
redefined.is_none(),
arms_loc,
loc,
"Match work queue went awry"
);
}
Expand All @@ -251,7 +251,7 @@ pub(super) fn compile_match(
hlir_context: context,
output_type: result_type,
arms: &arms,
arms_loc,
arms_loc: loc,
results: &mut compilation_results,
};
let match_exp = resolve_result(&mut resolution_context, &init_subject, match_start);
Expand Down
16 changes: 11 additions & 5 deletions external-crates/move/crates/move-compiler/src/shared/matching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ pub struct PatternArm {
#[derive(Clone, Debug)]
pub struct PatternMatrix {
pub tys: Vec<Type>,
pub loc: Loc,
pub patterns: Vec<PatternArm>,
}

Expand Down Expand Up @@ -445,6 +446,7 @@ impl PatternMatrix {
/// index for all of them.
pub fn from<const AFTER_TYPING: bool, MC: MatchContext<AFTER_TYPING>>(
context: &mut MC,
loc: Loc,
subject_ty: Type,
arms: Vec<T::MatchArm>,
) -> (PatternMatrix, Vec<T::Exp>) {
Expand Down Expand Up @@ -484,7 +486,7 @@ impl PatternMatrix {
});
}
}
(PatternMatrix { tys, patterns }, rhss)
(PatternMatrix { tys, loc, patterns }, rhss)
}

pub fn is_empty(&self) -> bool {
Expand Down Expand Up @@ -541,6 +543,7 @@ impl PatternMatrix {
) -> (Binders, PatternMatrix) {
let mut patterns = vec![];
let mut bindings = vec![];
let loc = self.loc;
for entry in &self.patterns {
if let Some((mut new_bindings, arm)) =
entry.specialize_variant(context, ctor_name, &arg_types)
Expand All @@ -554,7 +557,7 @@ impl PatternMatrix {
.cloned()
.chain(self.tys.clone().into_iter().skip(1))
.collect::<Vec<_>>();
let matrix = PatternMatrix { tys, patterns };
let matrix = PatternMatrix { tys, loc, patterns };
(bindings, matrix)
}

Expand All @@ -565,6 +568,7 @@ impl PatternMatrix {
) -> (Binders, PatternMatrix) {
let mut patterns = vec![];
let mut bindings = vec![];
let loc = self.loc;
for entry in &self.patterns {
if let Some((mut new_bindings, arm)) = entry.specialize_struct(context, &arg_types) {
bindings.append(&mut new_bindings);
Expand All @@ -576,35 +580,37 @@ impl PatternMatrix {
.cloned()
.chain(self.tys.clone().into_iter().skip(1))
.collect::<Vec<_>>();
let matrix = PatternMatrix { tys, patterns };
let matrix = PatternMatrix { tys, loc, patterns };
(bindings, matrix)
}

pub fn specialize_literal(&self, lit: &Value) -> (Binders, PatternMatrix) {
let mut patterns = vec![];
let mut bindings = vec![];
let loc = self.loc;
for entry in &self.patterns {
if let Some((mut new_bindings, arm)) = entry.specialize_literal(lit) {
bindings.append(&mut new_bindings);
patterns.push(arm)
}
}
let tys = self.tys[1..].to_vec();
let matrix = PatternMatrix { tys, patterns };
let matrix = PatternMatrix { tys, loc, patterns };
(bindings, matrix)
}

pub fn specialize_default(&self) -> (Binders, PatternMatrix) {
let mut patterns = vec![];
let mut bindings = vec![];
let loc = self.loc;
for entry in &self.patterns {
if let Some((mut new_bindings, arm)) = entry.specialize_default() {
bindings.append(&mut new_bindings);
patterns.push(arm)
}
}
let tys = self.tys[1..].to_vec();
let matrix = PatternMatrix { tys, patterns };
let matrix = PatternMatrix { tys, loc, patterns };
(bindings, matrix)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,13 @@ struct MatchCompiler<'ctx, 'env> {
impl TypingVisitorContext for MatchCompiler<'_, '_> {
fn visit_exp_custom(&mut self, exp: &mut T::Exp) -> bool {
use T::UnannotatedExp_ as E;
let eloc = exp.exp.loc;
if let E::Match(subject, arms) = &exp.exp.value {
debug_print!(self.context.debug.match_counterexample,
("subject" => subject),
(lines "arms" => &arms.value)
);
if invalid_match(self.context, subject, arms) {
if invalid_match(self.context, eloc, subject, arms) {
debug_print!(
self.context.debug.match_counterexample,
(msg "counterexample found")
Expand Down Expand Up @@ -93,12 +94,13 @@ pub fn function_body_(context: &mut Context, b_: &mut T::FunctionBody_) {
/// error.
fn invalid_match(
context: &mut Context,
loc: Loc,
subject: &T::Exp,
arms: &Spanned<Vec<T::MatchArm>>,
) -> bool {
let arms_loc = arms.loc;
let (pattern_matrix, _arms) =
PatternMatrix::from(context, subject.ty.clone(), arms.value.clone());
PatternMatrix::from(context, loc, subject.ty.clone(), arms.value.clone());

let mut counterexample_matrix = pattern_matrix.clone();
let has_guards = counterexample_matrix.has_guards();
Expand Down Expand Up @@ -547,9 +549,17 @@ fn find_counterexample_impl(
None
}
}
} else {
assert!(matrix.is_empty());
} else if matrix.is_empty() {
Some(make_wildcards(arity as usize))
} else {
// An error case: no entry on the fringe but no
if !context.env.has_errors() {
context.env.add_diag(ice!((
matrix.loc,
"Non-empty matrix with non errors but no type"
)));
}
None
};
debug_print!(context.debug.match_counterexample, (opt "result" => &result; sdbg));
result
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
error[E04016]: too few arguments
┌─ tests/move_2024/matching/counterexample_malformed_match.move:18:9
18 │ SomeEnum::PositionalFields(_, ) => (),
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Missing pattern for field '1' in 'a::m::SomeEnum::PositionalFields'

Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
module a::m;

public struct SomeStruct has drop {
some_field: u64,
}

public enum SomeEnum has drop {
PositionalFields(u64, SomeStruct),
}

public fun match_variant(s: SomeStruct) {
use a::m::SomeEnum as SE;
let e = SE::PositionalFields(42, s);
match (e) {
SomeEnum::PositionalFields(num, s) => {
num + s.some_field;
},
SomeEnum::PositionalFields(_, ) => (),
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
error[E03010]: unbound field
┌─ tests/move_2024/matching/counterexample_malformed_match_2.move:18:9
18 │ SomeEnum::PositionalFields(_, _, _) => (),
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Unbound field '2' in 'a::m::SomeEnum::PositionalFields'

Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
module a::m;

public struct SomeStruct has drop {
some_field: u64,
}

public enum SomeEnum has drop {
PositionalFields(u64, SomeStruct),
}

public fun match_variant(s: SomeStruct) {
use a::m::SomeEnum as SE;
let e = SE::PositionalFields(42, s);
match (e) {
SomeEnum::PositionalFields(num, s) => {
num + s.some_field;
},
SomeEnum::PositionalFields(_, _, _) => (),
}
}

0 comments on commit 32d0433

Please sign in to comment.