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.

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

fix: Parsing nested generics #1319

Merged
merged 8 commits into from
May 8, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
5 changes: 5 additions & 0 deletions crates/nargo_cli/tests/test_data/generics/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,9 @@ fn main(x: Field, y: Field) {

// Expected type error
// assert(bar2.get_other() == bar2.other);

let one = x;
let two = y;
let nested_generics: Bar<Bar<Field>> = Bar { one, two, other: Bar { one, two, other: 0 } };
assert(nested_generics.other.other == bar1.get_other());
}
2 changes: 1 addition & 1 deletion crates/noirc_driver/src/contract.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::program::{deserialize_circuit, serialize_circuit};
use acvm::acir::circuit::Circuit;
use noirc_abi::Abi;
use serde::{Deserialize, Serialize};
use crate::program::{serialize_circuit, deserialize_circuit};

/// Describes the types of smart contract functions that are allowed.
/// Unlike the similar enum in noirc_frontend, 'open' and 'unconstrained'
Expand Down
5 changes: 2 additions & 3 deletions crates/noirc_frontend/src/lexer/lexer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,8 @@ impl<'a> Lexer<'a> {
if self.peek_char_is('=') {
self.next_char();
Ok(Token::GreaterEqual.into_span(start, start + 1))
} else if self.peek_char_is('>') {
self.next_char();
Ok(Token::ShiftRight.into_span(start, start + 1))
// Note: There is deliberately now case for RightShift. We always lex >> as
jfecher marked this conversation as resolved.
Show resolved Hide resolved
// two separate Greater tokens to help the parser parse nested generic types.
} else {
Ok(prev_token.into_single_span(start))
}
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_frontend/src/parser/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ impl chumsky::Error<Token> for ParserError {
self.reason = other.reason;
}

assert_eq!(self.span, other.span);
// assert_eq!(self.span, other.span);
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
self
}
}
38 changes: 30 additions & 8 deletions crates/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,9 +517,22 @@ where
)
}

/// Parse an assignment operator `=` optionally prefixed by a binary operator for a combined
/// assign statement shorthand. Notably, this must handle a few corner cases with how `>>` is
/// lexed as two separate greater-than operators rather than a single right-shift.
fn assign_operator() -> impl NoirParser<Token> {
let shorthand_operators = Token::assign_shorthand_operators();
let shorthand_syntax = one_of(shorthand_operators).then_ignore(just(Token::Assign));
// We need to explicitly check for right_shift here since it is actually
// two separate greater-than operators.
let shorthand_operators = right_shift_operator().or(one_of(shorthand_operators));
let shorthand_syntax = shorthand_operators.then_ignore(just(Token::Assign));

// Since >> is lexed as two separte greater-thans, >>= is lexed as > >=, so
jfecher marked this conversation as resolved.
Show resolved Hide resolved
// we need to account for that case here as well.
let right_shift_fix =
just(Token::Greater).then(just(Token::GreaterEqual)).map(|_| Token::ShiftRight);

let shorthand_syntax = shorthand_syntax.or(right_shift_fix);
just(Token::Assign).or(shorthand_syntax)
}

Expand Down Expand Up @@ -726,14 +739,23 @@ fn create_infix_expression(lhs: Expression, (operator, rhs): (BinaryOp, Expressi
Expression { span, kind: ExpressionKind::Infix(infix) }
}

// Right-shift (>>) is issued as two separate > tokens by the lexer as this makes it easier
// to parse nested generic types. For normal expressions however, it means we have to manually
// parse two greater-than tokens as a single right-shift here.
fn right_shift_operator() -> impl NoirParser<Token> {
just(Token::Greater).then(just(Token::Greater)).map(|_| Token::ShiftRight)
}

fn operator_with_precedence(precedence: Precedence) -> impl NoirParser<Spanned<BinaryOpKind>> {
filter_map(move |span, token: Token| {
if Precedence::token_precedence(&token) == Some(precedence) {
Ok(token.try_into_binary_op(span).unwrap())
} else {
Err(ParserError::expected_label("binary operator".to_string(), token, span))
}
})
right_shift_operator()
.or(any()) // Parse any single token, we're validating it as an operator next
.try_map(move |token, span| {
if Precedence::token_precedence(&token) == Some(precedence) {
Ok(token.try_into_binary_op(span).unwrap())
} else {
Err(ParserError::expected_label("binary operator".to_string(), token, span))
}
})
}

fn term<'a, P>(expr_parser: P) -> impl NoirParser<Expression> + 'a
Expand Down