Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

formatter continued 01 #22

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

formatter continued 01 #22

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
68f1d3c
Parse role anotations
FredTheDino Mar 5, 2024
b90597f
Thick arrows are operators too
FredTheDino Mar 5, 2024
49326f8
Underscores are starts to expressions
FredTheDino Mar 5, 2024
06dc230
Mitigate infinet loops from expression parse errors
FredTheDino Mar 5, 2024
8c5af16
Parse newtypes
FredTheDino Mar 5, 2024
ab1c8c3
Update tests - these all look fairly right
FredTheDino Mar 5, 2024
2c5713f
Fix bug with type-parsing
FredTheDino Mar 5, 2024
b485be4
Update all tests and fix a bug I introduced in the `one_or_more` func…
FredTheDino Mar 5, 2024
a3008bd
Make the tests a bit more robust
FredTheDino Mar 5, 2024
5c71117
Add more information to the infinet loop-crash
FredTheDino Mar 5, 2024
e65e5da
Attempt to parse derive declarations
FredTheDino Mar 5, 2024
f3243d1
Update the tests - not sure if these are right
FredTheDino Mar 5, 2024
f2d1000
Clear error string is better than empty string
FredTheDino Mar 5, 2024
a0e2caf
Fix the crash
FredTheDino Mar 5, 2024
0e367b0
Write something that kinda looks like a syntax tree from the formatter
FredTheDino Mar 5, 2024
4da6f57
Parse a bunch of files instead
FredTheDino Mar 5, 2024
704daf5
Remove `=>` from being a valid operator
FredTheDino Mar 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/lexing/src/lexer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,10 @@ impl<'a> Lexer<'a> {
"true" => SyntaxKind::LiteralTrue,
"type" => SyntaxKind::TypeKw,
"where" => SyntaxKind::WhereKw,
"role" => SyntaxKind::RoleKw,
"nominal" => SyntaxKind::NominalKw,
"representational" => SyntaxKind::RepresentationalKw,
"phantom" => SyntaxKind::PhantomKw,
_ => SyntaxKind::Lower,
};
self.lexed.push(kind, position, None)
Expand Down
4 changes: 3 additions & 1 deletion crates/parsing/src/grammar/combinators.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ use crate::parser::Parser;
pub(super) fn one_or_more(parser: &mut Parser, rule: impl Fn(&mut Parser) -> bool) -> bool {
let mut marker = parser.start();
let mut at_least_one = false;
let mut last_index = 0;
loop {
if !rule(parser) {
if parser.index() == last_index || !rule(parser) {
break;
}
last_index = parser.index();
at_least_one = true;
}
if at_least_one {
Expand Down
68 changes: 45 additions & 23 deletions crates/parsing/src/grammar/rules.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ fn expr_binding(parser: &mut Parser, separator: SyntaxKind) {
expr_where(parser);
marker.end(parser, SyntaxKind::UnconditionalBinding);
} else {
// NOTE[et]: We get an infinet loop here since `expr_guarded` needs to beable to match an empty expression, so we just abort this loop if we have looped. It's a crude solution but it works to eliminate infinet loops in the parser. This logic lies inside the `one_or_more` parser
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an example file that can cause this infinite loop?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A minimum reproduction that I conjured is:

b | if 0 else 1 then 2 = 0

This fails due to how expr_if is defined:

fn expr_if(parser: &mut Parser) {
    let mut marker = parser.start();

    parser.expect(SyntaxKind::IfKw);
    expr_0(parser);

    parser.expect(SyntaxKind::ThenKw);
    expr_0(parser);

    parser.expect(SyntaxKind::ElseKw);
    expr_0(parser);

    marker.end(parser, SyntaxKind::IfThenElseExpression);
}

More specifically, expect is basically eat but it emits an error if it isn't able to consume the current token. The new expect_recover function is more suitable for this, where it eats the current token indiscriminately while also emitting an error node for the CST.

That being said, loop-safety in the parser is definitely something we should strive for; checking if the parser made progress is a good solution, and I think we should also look into "recovering" as much as possible in the parsing rules.

one_or_more(parser, |parser| {
// FIXME: is there an advantage if we use positives here?
if parser.current().is_end() || parser.at(separator) {
Expand Down Expand Up @@ -787,6 +788,7 @@ fn at_type_start(parser: &mut Parser) -> bool {
| SyntaxKind::Minus
| SyntaxKind::LeftParenthesis
| SyntaxKind::LeftCurly
| SyntaxKind::Underscore
)
}

Expand Down Expand Up @@ -1654,35 +1656,55 @@ fn annotation_or_type_declaration(parser: &mut Parser) {
let mut marker = parser.start();

parser.expect(SyntaxKind::TypeKw);
if matches!(parser.current(), SyntaxKind::Upper) {
name(parser, SyntaxKind::Upper);
} else {
parser.error_recover("expected a name");
}

if parser.at(SyntaxKind::Colon2) {
parser.consume();
type_0(parser);
marker.end(parser, SyntaxKind::TypeDeclarationAnnotation);
} else {
zero_or_more(parser, |parser| {
if at_type_var_binding_plain(parser) {
type_var_binding_plain(parser);
true
match parser.current() {
SyntaxKind::Upper => {
name(parser, SyntaxKind::Upper);
if parser.at(SyntaxKind::Colon2) {
parser.consume();
type_0(parser);
marker.end(parser, SyntaxKind::TypeDeclarationAnnotation);
} else {
false
}
});
zero_or_more(parser, |parser| {
if at_type_var_binding_plain(parser) {
type_var_binding_plain(parser);
true
} else {
false
}
});

if parser.at(SyntaxKind::Equal) {
parser.expect(SyntaxKind::Equal);
type_0(parser);
}
if parser.at(SyntaxKind::Equal) {
parser.expect(SyntaxKind::Equal);
type_0(parser);
}

marker.end(parser, SyntaxKind::TypeDeclaration);
marker.end(parser, SyntaxKind::TypeDeclaration);
}
}
SyntaxKind::RoleKw => {
parser.consume();
name(parser, SyntaxKind::Upper);
loop {
if !is_role_kind(parser) {
break;
}
parser.consume()
}
marker.end(parser, SyntaxKind::TypeDeclarationRole);
}
_ => {
parser.error_recover("expected type statement");
}
}
}

fn is_role_kind(parser: &mut Parser) -> bool {
matches!(
parser.current(),
SyntaxKind::NominalKw | SyntaxKind::RepresentationalKw | SyntaxKind::PhantomKw
)
}

// 'foreign' 'import' 'lower' '::' type_0
// | 'foreign' 'import' 'data' 'upper' '::' type_0
fn foreign_import_declaration(parser: &mut Parser) {
Expand Down
4 changes: 4 additions & 0 deletions crates/parsing/src/parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ impl<'a> Parser<'a> {
self.nth(offset) == kind
}

pub(crate) fn index(&self) -> usize {
self.index
}

pub(crate) fn current(&self) -> SyntaxKind {
self.nth(0)
}
Expand Down
7 changes: 6 additions & 1 deletion crates/syntax/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,13 @@ pub enum SyntaxKind {
ForallKw,

TypeDeclarationAnnotation,
TypeDeclarationRole,
TypeDeclaration,
TypeKw,
RoleKw,
NominalKw,
RepresentationalKw,
PhantomKw,

ClassSignature,
ClassDeclaration,
Expand Down Expand Up @@ -285,7 +290,7 @@ impl SyntaxKind {
}

pub fn is_operator(&self) -> bool {
matches!(self, Self::Operator | Self::Minus | Self::Colon)
matches!(self, Self::Operator | Self::Minus | Self::Colon | Self::LeftThickArrow | Self::RightThickArrow)
}

pub fn is_reserved_operator(&self) -> bool {
Expand Down