Skip to content

Commit

Permalink
parser: consider physically separated chunks as separate singular exp…
Browse files Browse the repository at this point in the history
…ressions

This commit changes the parser's handling of singular expressions,
i.e. ones that don't have any operators between them. Previously, the
parser would eagerly continue parsing a singular expression until it
matched the legal structure of an expression. Whilst this was legal,
and handled accordingly at later phases of compilation, it certainly
created odd (and unexpected) behaviour in the parser.

Now, we consider singular expressions as a collection of tokens that
must be "physically" connected. For example, before hand we consider:

```rs
foo
()

// Or

foo ()
```

as a call to the function `foo`, however, now we consider them as
separate expressions. Now, calling foo can only be done with:
```rs
foo()
```

In addition to that, we also fix some newly "detected" issues in
existing tests.
  • Loading branch information
feds01 committed Dec 9, 2024
1 parent 9bd6186 commit f8a031f
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 16 deletions.
15 changes: 13 additions & 2 deletions compiler/hash-parser/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,11 +357,18 @@ impl AstGen<'_> {
pub(crate) fn parse_singular_expr(
&mut self,
mut subject: AstNode<Expr>,
subject_span: ByteRange,
mut subject_span: ByteRange,
) -> ParseResult<AstNode<Expr>> {
// so here we need to peek to see if this is either a index_access, field access
// or a function call...
while let Some(token) = self.peek() {
// if there exists a space between the `subject` and the
// next fragment of the expression, we treat them as explicitly
// non-singular expressions.
if !token.span.is_right_before(subject_span) {
break;
}

subject = match token.kind {
// Property access or method call
TokenKind::Dot => self.parse_property_access(subject, subject_span)?,
Expand All @@ -382,7 +389,11 @@ impl AstGen<'_> {
// Function call
TokenKind::Tree(Delimiter::Paren, _) => self.parse_call(subject, subject_span)?,
_ => break,
}
};

// We need to adjust the subject_span so we can compute whether
// we're still "physically" connected to the end of the expression.
subject_span = subject_span.join(self.previous_pos())
}

Ok(subject)
Expand Down
14 changes: 14 additions & 0 deletions compiler/hash-source/src/location.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,20 @@ impl ByteRange {
pub fn into_location(self, source_id: SourceId) -> Span {
Span::new(self, source_id)
}

/// Check if one span intersects with another one.
///
/// We check whether the `current` token touches the other token, if it
/// does, then we consider it to be intersecting.
/// ```
///
/// (current token)
/// (other token)
/// ```
#[inline(always)]
pub fn is_right_before(&self, other: Self) -> bool {
self.start() == (other.end() + 1)
}
}

impl Default for ByteRange {
Expand Down
4 changes: 2 additions & 2 deletions tests/cases/parser/blocks/empty_for_loop.hash
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ main := () => {
// Test that the parser shouldn't get confused between a struct literal and a for block
for x in xs {}

// Test that struct literals are allowed in parenthesees...
for x in (Struct ( xs )) {}
// Test that struct literals are allowed in parentheses...
for x in (Struct( xs )) {}
}
2 changes: 1 addition & 1 deletion tests/cases/parser/constructors/struct_defn.hash
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Point := struct(

// Test that structs can be initialised with default field value and types.
Triangle := struct(
x: Point = Point (x = 0, y = 0),
x: Point = Point(x = 0, y = 0),
y: Point,
z := Point( x = 0, y = 0), // Type should be inferred from default value?
);
Expand Down
2 changes: 1 addition & 1 deletion tests/cases/parser/patterns/list_patterns.hash
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
[a, b] := foo();
[] := foo();

[x, Struct ( name )] = *mu();
[x, Struct(name)] = *mu();

[(a, 3) | (a, 2), (c, d), (r, m)] := foo();

4 changes: 2 additions & 2 deletions tests/cases/typecheck/pat_binds/548.hash
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
main := () => {
// ~ERROR: variable `c` not declared in all patterns
// ~ERROR: variable `d` not declared in all patterns
(c | d) := 2;
(c | d) := 2;

k := (1, 2)

match k {
(a, b) | (t, a) => {}
(a, b) | (t, a) => {},
(2, 3) | (a, b) => {}
}
}
42 changes: 34 additions & 8 deletions tests/cases/typecheck/pat_binds/548.stderr
Original file line number Diff line number Diff line change
@@ -1,51 +1,77 @@
error[0081]: variable `c` is not bound in all patterns
--> $DIR/548.hash:6:10
5 | // ~ERROR: variable `d` not declared in all patterns
6 | (c | d) := 2;
6 | (c | d) := 2;
| ^ pattern doesn't bind `c`
7 |

--> $DIR/548.hash:6:6
5 | // ~ERROR: variable `d` not declared in all patterns
6 | (c | d) := 2;
6 | (c | d) := 2;
| ^ variable not in all patterns
7 |

error[0081]: variable `d` is not bound in all patterns
--> $DIR/548.hash:6:6
5 | // ~ERROR: variable `d` not declared in all patterns
6 | (c | d) := 2;
6 | (c | d) := 2;
| ^ pattern doesn't bind `d`
7 |

--> $DIR/548.hash:6:10
5 | // ~ERROR: variable `d` not declared in all patterns
6 | (c | d) := 2;
6 | (c | d) := 2;
| ^ variable not in all patterns
7 |

error[0081]: variable `b` is not bound in all patterns
--> $DIR/548.hash:11:18
10 | match k {
11 | (a, b) | (t, a) => {}
11 | (a, b) | (t, a) => {},
| ^^^^^^ pattern doesn't bind `b`
12 | (2, 3) | (a, b) => {}

--> $DIR/548.hash:11:13
10 | match k {
11 | (a, b) | (t, a) => {}
11 | (a, b) | (t, a) => {},
| ^ variable not in all patterns
12 | (2, 3) | (a, b) => {}

error[0081]: variable `t` is not bound in all patterns
--> $DIR/548.hash:11:9
10 | match k {
11 | (a, b) | (t, a) => {}
11 | (a, b) | (t, a) => {},
| ^^^^^^ pattern doesn't bind `t`
12 | (2, 3) | (a, b) => {}

--> $DIR/548.hash:11:19
10 | match k {
11 | (a, b) | (t, a) => {}
11 | (a, b) | (t, a) => {},
| ^ variable not in all patterns
12 | (2, 3) | (a, b) => {}

error[0081]: variable `a` is not bound in all patterns
--> $DIR/548.hash:12:9
11 | (a, b) | (t, a) => {},
12 | (2, 3) | (a, b) => {}
| ^^^^^^ pattern doesn't bind `a`
13 | }

--> $DIR/548.hash:12:19
11 | (a, b) | (t, a) => {},
12 | (2, 3) | (a, b) => {}
| ^ variable not in all patterns
13 | }

error[0081]: variable `b` is not bound in all patterns
--> $DIR/548.hash:12:9
11 | (a, b) | (t, a) => {},
12 | (2, 3) | (a, b) => {}
| ^^^^^^ pattern doesn't bind `b`
13 | }

--> $DIR/548.hash:12:22
11 | (a, b) | (t, a) => {},
12 | (2, 3) | (a, b) => {}
| ^ variable not in all patterns
13 | }

0 comments on commit f8a031f

Please sign in to comment.