Skip to content

Commit

Permalink
fix: don't fail parsing macro if there are parser warnings (#6969)
Browse files Browse the repository at this point in the history
  • Loading branch information
asterite authored Jan 7, 2025
1 parent 38d429e commit 7f0067c
Show file tree
Hide file tree
Showing 8 changed files with 106 additions and 61 deletions.
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 @@ -1322,7 +1322,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
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 @@ use crate::{
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 @@ -158,7 +159,7 @@ impl Value {

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 @@ impl Value {
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 @@ impl Value {
}
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 @@ impl Value {

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 @@ impl Value {
| 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 @@ impl Value {
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 @@ pub(crate) fn unwrap_rc<T: Clone>(rc: Rc<T>) -> T {

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 @@ where
{
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

0 comments on commit 7f0067c

Please sign in to comment.