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: don't fail parsing macro if there are parser warnings #6969

Merged
merged 2 commits into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/elaborator/comptime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ impl<'context> Elaborator<'context> {

if value != Value::Unit {
let items = value
.into_top_level_items(location, self.interner)
.into_top_level_items(location, self)
.map_err(|error| error.into_compilation_error_pair())?;

self.add_items(items, generated_items, location);
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/elaborator/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,7 @@ impl<'context> Elaborator<'context> {
};

let location = Location::new(span, self.file);
match value.into_expression(self.interner, location) {
match value.into_expression(self, location) {
Ok(new_expr) => {
// At this point the Expression was already elaborated and we got a Value.
// We'll elaborate this value turned into Expression to inline it and get
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir/comptime/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@
}
} else {
let name = self.elaborator.interner.function_name(&function);
unreachable!("Non-builtin, lowlevel or oracle builtin fn '{name}'")

Check warning on line 255 in compiler/noirc_frontend/src/hir/comptime/interpreter.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lowlevel)
}
}

Expand Down Expand Up @@ -1046,7 +1046,7 @@
}

/// Generate matches for bit shifting, which in Noir only accepts `u8` for RHS.
macro_rules! match_bitshift {

Check warning on line 1049 in compiler/noirc_frontend/src/hir/comptime/interpreter.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (bitshift)
(($lhs_value:ident as $lhs:ident $op:literal $rhs_value:ident as $rhs:ident) => $expr:expr) => {
match_values! {
($lhs_value as $lhs $op $rhs_value as $rhs) {
Expand Down Expand Up @@ -1116,10 +1116,10 @@
BinaryOpKind::Xor => match_bitwise! {
(lhs_value as lhs "^" rhs_value as rhs) => lhs ^ rhs
},
BinaryOpKind::ShiftRight => match_bitshift! {

Check warning on line 1119 in compiler/noirc_frontend/src/hir/comptime/interpreter.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (bitshift)
(lhs_value as lhs ">>" rhs_value as rhs) => lhs.checked_shr(rhs.into())
},
BinaryOpKind::ShiftLeft => match_bitshift! {

Check warning on line 1122 in compiler/noirc_frontend/src/hir/comptime/interpreter.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (bitshift)
(lhs_value as lhs "<<" rhs_value as rhs) => lhs.checked_shl(rhs.into())
},
BinaryOpKind::Modulo => match_integer! {
Expand Down Expand Up @@ -1322,7 +1322,7 @@
let bindings = unwrap_rc(bindings);
let mut result = self.call_function(function_id, arguments, bindings, location)?;
if call.is_macro_call {
let expr = result.into_expression(self.elaborator.interner, location)?;
let expr = result.into_expression(self.elaborator, location)?;
let expr = self.elaborate_in_function(self.current_function, |elaborator| {
elaborator.elaborate_expression(expr).0
});
Expand Down
43 changes: 22 additions & 21 deletions compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use crate::{
Pattern, Signedness, Statement, StatementKind, UnaryOp, UnresolvedType, UnresolvedTypeData,
Visibility,
},
elaborator::Elaborator,
hir::{
comptime::{
errors::IResult,
Expand All @@ -32,9 +33,11 @@ use crate::{
def_collector::dc_crate::CollectedItems,
def_map::ModuleDefId,
},
hir_def::expr::{HirExpression, HirLiteral},
hir_def::function::FunctionBody,
hir_def::{self},
hir_def::{
self,
expr::{HirExpression, HirLiteral},
function::FunctionBody,
},
node_interner::{DefinitionKind, NodeInterner, TraitImplKind},
parser::{Parser, StatementOrExpressionOrLValue},
token::{Attribute, Token},
Expand Down Expand Up @@ -158,7 +161,7 @@ impl<'local, 'context> Interpreter<'local, 'context> {
"modulus_le_bits" => modulus_le_bits(arguments, location),
"modulus_le_bytes" => modulus_le_bytes(arguments, location),
"modulus_num_bits" => modulus_num_bits(arguments, location),
"quoted_as_expr" => quoted_as_expr(interner, arguments, return_type, location),
"quoted_as_expr" => quoted_as_expr(self.elaborator, arguments, return_type, location),
"quoted_as_module" => quoted_as_module(self, arguments, return_type, location),
"quoted_as_trait_constraint" => quoted_as_trait_constraint(self, arguments, location),
"quoted_as_type" => quoted_as_type(self, arguments, location),
Expand Down Expand Up @@ -676,15 +679,19 @@ fn slice_insert(

// fn as_expr(quoted: Quoted) -> Option<Expr>
fn quoted_as_expr(
interner: &NodeInterner,
elaborator: &mut Elaborator,
arguments: Vec<(Value, Location)>,
return_type: Type,
location: Location,
) -> IResult<Value> {
let argument = check_one_argument(arguments, location)?;

let result =
parse(interner, argument, Parser::parse_statement_or_expression_or_lvalue, "an expression");
let result = parse(
elaborator,
argument,
Parser::parse_statement_or_expression_or_lvalue,
"an expression",
);

let value =
result.ok().map(
Expand All @@ -709,13 +716,9 @@ fn quoted_as_module(
) -> IResult<Value> {
let argument = check_one_argument(arguments, location)?;

let path = parse(
interpreter.elaborator.interner,
argument,
Parser::parse_path_no_turbofish_or_error,
"a path",
)
.ok();
let path =
parse(interpreter.elaborator, argument, Parser::parse_path_no_turbofish_or_error, "a path")
.ok();
let option_value = path.and_then(|path| {
let module = interpreter
.elaborate_in_function(interpreter.current_function, |elaborator| {
Expand All @@ -735,7 +738,7 @@ fn quoted_as_trait_constraint(
) -> IResult<Value> {
let argument = check_one_argument(arguments, location)?;
let trait_bound = parse(
interpreter.elaborator.interner,
interpreter.elaborator,
argument,
Parser::parse_trait_bound_or_error,
"a trait constraint",
Expand All @@ -756,8 +759,7 @@ fn quoted_as_type(
location: Location,
) -> IResult<Value> {
let argument = check_one_argument(arguments, location)?;
let typ =
parse(interpreter.elaborator.interner, argument, Parser::parse_type_or_error, "a type")?;
let typ = parse(interpreter.elaborator, argument, Parser::parse_type_or_error, "a type")?;
let typ = interpreter
.elaborate_in_function(interpreter.current_function, |elab| elab.resolve_type(typ));
Ok(Value::Type(typ))
Expand Down Expand Up @@ -2453,7 +2455,7 @@ fn function_def_set_parameters(
)?;
let parameter_type = get_type((tuple.pop().unwrap(), parameters_argument_location))?;
let parameter_pattern = parse(
interpreter.elaborator.interner,
interpreter.elaborator,
(tuple.pop().unwrap(), parameters_argument_location),
Parser::parse_pattern_or_error,
"a pattern",
Expand Down Expand Up @@ -2570,12 +2572,11 @@ fn module_add_item(
) -> IResult<Value> {
let (self_argument, item) = check_two_arguments(arguments, location)?;
let module_id = get_module(self_argument)?;
let module_data = interpreter.elaborator.get_module(module_id);

let parser = Parser::parse_top_level_items;
let top_level_statements =
parse(interpreter.elaborator.interner, item, parser, "a top-level item")?;
let top_level_statements = parse(interpreter.elaborator, item, parser, "a top-level item")?;

let module_data = interpreter.elaborator.get_module(module_id);
interpreter.elaborate_in_module(module_id, module_data.location.file, |elaborator| {
let mut generated_items = CollectedItems::default();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ use acvm::FieldElement;
use iter_extended::try_vecmap;
use noirc_errors::Location;

use crate::elaborator::Elaborator;
use crate::hir::comptime::display::tokens_to_string;
use crate::hir::comptime::value::add_token_spans;
use crate::lexer::Lexer;
use crate::parser::Parser;
use crate::parser::{Parser, ParserError};
use crate::{
ast::{
BlockExpression, ExpressionKind, Ident, IntegerBitSize, LValue, Pattern, Signedness,
Expand Down Expand Up @@ -493,7 +494,7 @@ pub(super) fn lex(input: &str) -> Vec<Token> {
}

pub(super) fn parse<'a, T, F>(
interner: &NodeInterner,
elaborator: &mut Elaborator,
(value, location): (Value, Location),
parser: F,
rule: &'static str,
Expand All @@ -503,7 +504,12 @@ where
{
let tokens = get_quoted((value, location))?;
let quoted = add_token_spans(tokens.clone(), location.span);
parse_tokens(tokens, quoted, interner, location, parser, rule)
let (result, warnings) =
parse_tokens(tokens, quoted, elaborator.interner, location, parser, rule)?;
for warning in warnings {
elaborator.errors.push((warning.into(), location.file));
}
Ok(result)
}

pub(super) fn parse_tokens<'a, T, F>(
Expand All @@ -513,7 +519,7 @@ pub(super) fn parse_tokens<'a, T, F>(
location: Location,
parsing_function: F,
rule: &'static str,
) -> IResult<T>
) -> IResult<(T, Vec<ParserError>)>
where
F: FnOnce(&mut Parser<'a>) -> T,
{
Expand Down
51 changes: 32 additions & 19 deletions compiler/noirc_frontend/src/hir/comptime/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
IntegerBitSize, LValue, Literal, Path, Pattern, Signedness, Statement, StatementKind,
UnresolvedType, UnresolvedTypeData,
},
elaborator::Elaborator,
hir::{def_map::ModuleId, type_check::generics::TraitGenerics},
hir_def::expr::{
HirArrayLiteral, HirConstructorExpression, HirExpression, HirIdent, HirLambda, HirLiteral,
Expand Down Expand Up @@ -97,8 +98,8 @@
Value::Expr(ExprValue::Statement(statement))
}

pub(crate) fn lvalue(lvaue: LValue) -> Self {

Check warning on line 101 in compiler/noirc_frontend/src/hir/comptime/value.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lvaue)
Value::Expr(ExprValue::LValue(lvaue))

Check warning on line 102 in compiler/noirc_frontend/src/hir/comptime/value.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lvaue)
}

pub(crate) fn pattern(pattern: Pattern) -> Self {
Expand Down Expand Up @@ -158,7 +159,7 @@

pub(crate) fn into_expression(
self,
interner: &mut NodeInterner,
elaborator: &mut Elaborator,
location: Location,
) -> IResult<Expression> {
let kind = match self {
Expand Down Expand Up @@ -212,22 +213,23 @@
ExpressionKind::Literal(Literal::Str(unwrap_rc(value)))
}
Value::Function(id, typ, bindings) => {
let id = interner.function_definition_id(id);
let id = elaborator.interner.function_definition_id(id);
let impl_kind = ImplKind::NotATraitMethod;
let ident = HirIdent { location, id, impl_kind };
let expr_id = interner.push_expr(HirExpression::Ident(ident, None));
interner.push_expr_location(expr_id, location.span, location.file);
interner.push_expr_type(expr_id, typ);
interner.store_instantiation_bindings(expr_id, unwrap_rc(bindings));
let expr_id = elaborator.interner.push_expr(HirExpression::Ident(ident, None));
elaborator.interner.push_expr_location(expr_id, location.span, location.file);
elaborator.interner.push_expr_type(expr_id, typ);
elaborator.interner.store_instantiation_bindings(expr_id, unwrap_rc(bindings));
ExpressionKind::Resolved(expr_id)
}
Value::Tuple(fields) => {
let fields = try_vecmap(fields, |field| field.into_expression(interner, location))?;
let fields =
try_vecmap(fields, |field| field.into_expression(elaborator, location))?;
ExpressionKind::Tuple(fields)
}
Value::Struct(fields, typ) => {
let fields = try_vecmap(fields, |(name, field)| {
let field = field.into_expression(interner, location)?;
let field = field.into_expression(elaborator, location)?;
Ok((Ident::new(unwrap_rc(name), location.span), field))
})?;

Expand All @@ -246,12 +248,12 @@
}
Value::Array(elements, _) => {
let elements =
try_vecmap(elements, |element| element.into_expression(interner, location))?;
try_vecmap(elements, |element| element.into_expression(elaborator, location))?;
ExpressionKind::Literal(Literal::Array(ArrayLiteral::Standard(elements)))
}
Value::Slice(elements, _) => {
let elements =
try_vecmap(elements, |element| element.into_expression(interner, location))?;
try_vecmap(elements, |element| element.into_expression(elaborator, location))?;
ExpressionKind::Literal(Literal::Slice(ArrayLiteral::Standard(elements)))
}
Value::Quoted(tokens) => {
Expand All @@ -262,12 +264,18 @@

let parser = Parser::for_tokens(tokens_to_parse);
return match parser.parse_result(Parser::parse_expression_or_error) {
Ok(expr) => Ok(expr),
Ok((expr, warnings)) => {
for warning in warnings {
elaborator.errors.push((warning.into(), location.file));
}

Ok(expr)
}
Err(mut errors) => {
let error = errors.swap_remove(0);
let file = location.file;
let rule = "an expression";
let tokens = tokens_to_string(tokens, interner);
let tokens = tokens_to_string(tokens, elaborator.interner);
Err(InterpreterError::FailedToParseMacro { error, file, tokens, rule })
}
};
Expand All @@ -293,7 +301,7 @@
| Value::Closure(..)
| Value::ModuleDefinition(_) => {
let typ = self.get_type().into_owned();
let value = self.display(interner).to_string();
let value = self.display(elaborator.interner).to_string();
return Err(InterpreterError::CannotInlineMacro { typ, value, location });
}
};
Expand Down Expand Up @@ -533,16 +541,16 @@
pub(crate) fn into_top_level_items(
self,
location: Location,
interner: &NodeInterner,
elaborator: &mut Elaborator,
) -> IResult<Vec<Item>> {
let parser = Parser::parse_top_level_items;
match self {
Value::Quoted(tokens) => {
parse_tokens(tokens, interner, parser, location, "top-level item")
parse_tokens(tokens, elaborator, parser, location, "top-level item")
}
_ => {
let typ = self.get_type().into_owned();
let value = self.display(interner).to_string();
let value = self.display(elaborator.interner).to_string();
Err(InterpreterError::CannotInlineMacro { value, typ, location })
}
}
Expand All @@ -556,7 +564,7 @@

fn parse_tokens<'a, T, F>(
tokens: Rc<Vec<Token>>,
interner: &NodeInterner,
elaborator: &mut Elaborator,
parsing_function: F,
location: Location,
rule: &'static str,
Expand All @@ -566,11 +574,16 @@
{
let parser = Parser::for_tokens(add_token_spans(tokens.clone(), location.span));
match parser.parse_result(parsing_function) {
Ok(expr) => Ok(expr),
Ok((expr, warnings)) => {
for warning in warnings {
elaborator.errors.push((warning.into(), location.file));
}
Ok(expr)
}
Err(mut errors) => {
let error = errors.swap_remove(0);
let file = location.file;
let tokens = tokens_to_string(tokens, interner);
let tokens = tokens_to_string(tokens, elaborator.interner);
Err(InterpreterError::FailedToParseMacro { error, file, tokens, rule })
}
}
Expand Down
22 changes: 8 additions & 14 deletions compiler/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,12 @@ impl<'a> Parser<'a> {
}

/// Invokes `parsing_function` (`parsing_function` must be some `parse_*` method of the parser)
/// and returns the result if the parser has no errors, and if the parser consumed all tokens.
/// and returns the result (and any warnings) if the parser has no errors, and if the parser consumed all tokens.
/// Otherwise returns the list of errors.
pub fn parse_result<T, F>(mut self, parsing_function: F) -> Result<T, Vec<ParserError>>
pub fn parse_result<T, F>(
mut self,
parsing_function: F,
) -> Result<(T, Vec<ParserError>), Vec<ParserError>>
where
F: FnOnce(&mut Parser<'a>) -> T,
{
Expand All @@ -140,23 +143,14 @@ impl<'a> Parser<'a> {
return Err(self.errors);
}

if self.errors.is_empty() {
Ok(item)
let all_warnings = self.errors.iter().all(|error| error.is_warning());
if all_warnings {
Ok((item, self.errors))
} else {
Err(self.errors)
}
}

/// Invokes `parsing_function` (`parsing_function` must be some `parse_*` method of the parser)
/// and returns the result if the parser has no errors, and if the parser consumed all tokens.
/// Otherwise returns None.
pub fn parse_option<T, F>(self, parsing_function: F) -> Option<T>
where
F: FnOnce(&mut Parser<'a>) -> Option<T>,
{
self.parse_result(parsing_function).unwrap_or_default()
}

/// Bumps this parser by one token. Returns the token that was previously the "current" token.
fn bump(&mut self) -> SpannedToken {
self.previous_token_span = self.current_token_span;
Expand Down
Loading
Loading