Skip to content

Commit

Permalink
Switch from Span to TermPos for patterns
Browse files Browse the repository at this point in the history
Patterns were annotated with a span, which must always be a well-defined
location in the source file. The implementation of full pattern matching
- i.e. allowing match expression to have arbirary patterns on the left
hand side of "=>" - showed that this is an issue, because the enum
contract is implemented using match, and thus generate a match
expression without a definite position.

This commit switches to optional position instead, delaying the fearful
`pos.unwrap()` to the latest moment possible, when we need to generate a
contract and build a `Label` during a pattern destructuring, which
shouldn't affect the contract-generated match expressions.
  • Loading branch information
yannham committed Feb 14, 2024
1 parent a262f73 commit 2a9448e
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 22 deletions.
20 changes: 12 additions & 8 deletions core/src/parser/grammar.lalrpop
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ Pattern: Pattern = {
Pattern {
alias,
data,
span: mk_span(src_id, l, r),
pos: mk_pos(src_id, l, r),
}
},
};
Expand Down Expand Up @@ -553,9 +553,13 @@ RecordPattern: RecordPattern = {
None => RecordPatternTail::Empty,
};

let span = mk_span(src_id, start, end);
let pattern = RecordPattern { patterns: field_pats, tail, span };
let pattern = RecordPattern {
patterns: field_pats,
tail,
pos: mk_pos(src_id, start, end)
};
pattern.check_dup()?;

Ok(pattern)
},
};
Expand All @@ -564,12 +568,12 @@ EnumPattern: EnumPattern = {
<start: @L> <tag: EnumTag> <end: @R> => EnumPattern {
tag,
pattern: None,
span: mk_span(src_id, start, end),
pos: mk_pos(src_id, start, end),
},
<start: @L> <tag: EnumTag> ".." "(" <pattern: Pattern> ")" <end: @R> => EnumPattern {
tag,
pattern: Some(Box::new(pattern)),
span: mk_span(src_id, start, end),
pos: mk_pos(src_id, start, end),
},
};

Expand All @@ -583,7 +587,7 @@ FieldPattern: FieldPattern = {
matched_id,
extra,
pattern,
span: mk_span(src_id, l, r),
pos: mk_pos(src_id, l, r),
}
},
<l: @L> <matched_id:Ident> <anns: SimpleFieldAnnot<FixedType>?> <default: DefaultAnnot?> <r: @R> => {
Expand All @@ -596,10 +600,10 @@ FieldPattern: FieldPattern = {
data: PatternData::Any(matched_id),
//unwrap(): the position of an parsed identifier should always
//be defined
span: matched_id.pos.unwrap(),
pos: matched_id.pos,
alias: None,
},
span: mk_span(src_id, l, r)
pos: mk_pos(src_id, l, r)
}
},
};
Expand Down
29 changes: 15 additions & 14 deletions core/src/term/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
label::Label,
mk_app,
parser::error::ParseError,
position::{RawSpan, TermPos},
position::TermPos,
stdlib::internals,
term::{
record::{Field, RecordAttrs, RecordData},
Expand Down Expand Up @@ -38,16 +38,16 @@ pub struct Pattern {
/// A potential alias for this pattern, capturing the whole matched value. In the source
/// language, an alias is introduced by `x @ <pattern>`, where `x` is an arbitrary identifier.
pub alias: Option<LocIdent>,
/// The span of the pattern in the source.
pub span: RawSpan,
/// The position of the pattern in the source.
pub pos: TermPos,
}

/// An enum pattern, including both an enum tag and an enum variant.
#[derive(Debug, PartialEq, Clone)]
pub struct EnumPattern {
pub tag: LocIdent,
pub pattern: Option<Box<Pattern>>,
pub span: RawSpan,
pub pos: TermPos,
}

/// A field pattern inside a record pattern. Every field can be annotated with a type, contracts or
Expand All @@ -64,7 +64,7 @@ pub struct FieldPattern {
/// sign, is parsed as `{foo=foo, bar=bar}`. In this case, `pattern.data` will be
/// [PatternData::Any].
pub pattern: Pattern,
pub span: RawSpan,
pub pos: TermPos,
}

/// The last match in a data structure pattern. This can either be a normal match, or an ellipsis
Expand Down Expand Up @@ -97,7 +97,7 @@ pub struct RecordPattern {
/// The tail of the pattern, indicating if the pattern is open, i.e. if it ended with an
/// ellipsis, capturing the rest or not.
pub tail: RecordPatternTail,
pub span: RawSpan,
pub pos: TermPos,
}

/// The tail of a record pattern which might capture the rest of the record.
Expand Down Expand Up @@ -213,8 +213,6 @@ impl ElaborateContract for Pattern {

impl ElaborateContract for EnumPattern {
fn elaborate_contract(&self) -> Option<LabeledType> {
let pos = TermPos::Original(self.span);

// TODO[adts]: it would be better to simply build a type like `[| 'tag arg |]` or `[| 'tag
// |]` and to rely on its derived contract. However, for the time being, the contract
// derived from enum variants isn't implemented yet.
Expand All @@ -226,14 +224,18 @@ impl ElaborateContract for EnumPattern {

let typ = Type {
typ: TypeF::Flat(contract),
pos,
pos: self.pos,
};

Some(LabeledType {
typ: typ.clone(),
label: Label {
typ: typ.into(),
span: self.span,
// [^unwrap-span]: We need the position to be defined here. Hopefully,
// contract-generating pattern are pattern used in destructuring, and destructuring
// patterns aren't currently generated by the Nickel interpreter. So we should only
// encounter user-written patterns here, which should have a position.
span: self.pos.unwrap(),
..Default::default()
},
})
Expand All @@ -242,8 +244,6 @@ impl ElaborateContract for EnumPattern {

impl ElaborateContract for RecordPattern {
fn elaborate_contract(&self) -> Option<LabeledType> {
let pos = TermPos::Original(self.span);

let typ = Type {
typ: TypeF::Flat(
Term::Record(RecordData::new(
Expand All @@ -259,14 +259,15 @@ impl ElaborateContract for RecordPattern {
))
.into(),
),
pos,
pos: self.pos,
};

Some(LabeledType {
typ: typ.clone(),
label: Label {
typ: typ.into(),
span: self.span,
// unwrap(): cf [^unwrap-span]
span: self.pos.unwrap(),
..Default::default()
},
})
Expand Down

0 comments on commit 2a9448e

Please sign in to comment.