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: Fix tokenization of unquoted types in macros #5326

Merged
merged 4 commits into from
Jun 25, 2024
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
6 changes: 6 additions & 0 deletions compiler/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
pub use type_alias::*;

use crate::{
node_interner::QuotedTypeId,
parser::{ParserError, ParserErrorReason},
token::IntType,
BinaryTypeOperator,
Expand Down Expand Up @@ -116,9 +117,13 @@
/*env:*/ Box<UnresolvedType>,
),

// The type of quoted code for metaprogramming

Check warning on line 120 in compiler/noirc_frontend/src/ast/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (metaprogramming)
Quoted(crate::QuotedType),

/// An already resolved type. These can only be parsed if they were present in the token stream
/// as a result of being spliced into a macro's token stream input.
Resolved(QuotedTypeId),

Unspecified, // This is for when the user declares a variable without specifying it's type
Error,
}
Expand Down Expand Up @@ -221,6 +226,7 @@
Error => write!(f, "error"),
Unspecified => write!(f, "unspecified"),
Parenthesized(typ) => write!(f, "({typ})"),
Resolved(_) => write!(f, "(resolved type)"),
}
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ impl<'context> Elaborator<'context> {
Type::MutableReference(Box::new(self.resolve_type_inner(*element, kind)))
}
Parenthesized(typ) => self.resolve_type_inner(*typ, kind),
Resolved(id) => self.interner.get_quoted_type(id).clone(),
};

if let Type::Struct(_, _) = resolved_type {
Expand Down
3 changes: 2 additions & 1 deletion compiler/noirc_frontend/src/hir/comptime/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@
.expect("all builtin functions must contain a function attribute which contains the opcode which it links to");

if let Some(builtin) = func_attrs.builtin() {
builtin::call_builtin(self.interner, builtin, arguments, location)
let builtin = builtin.clone();
builtin::call_builtin(self.interner, &builtin, arguments, location)
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
} else if let Some(foreign) = func_attrs.foreign() {
let item = format!("Comptime evaluation for foreign functions like {foreign}");
Err(InterpreterError::Unimplemented { item, location })
Expand All @@ -111,7 +112,7 @@
}
} else {
let name = self.interner.function_name(&function);
unreachable!("Non-builtin, lowlevel or oracle builtin fn '{name}'")

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

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lowlevel)
}
}

Expand Down
28 changes: 5 additions & 23 deletions compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,13 @@ use noirc_errors::Location;

use crate::{
hir::comptime::{errors::IResult, InterpreterError, Value},
lexer::Lexer,
macros_api::NodeInterner,
token::{SpannedToken, Token, Tokens},
QuotedType, Type,
};

pub(super) fn call_builtin(
interner: &NodeInterner,
interner: &mut NodeInterner,
name: &str,
arguments: Vec<(Value, Location)>,
location: Location,
Expand Down Expand Up @@ -124,7 +123,7 @@ fn type_def_generics(
/// fn fields(self) -> [(Quoted, Quoted)]
/// Returns (name, type) pairs of each field of this TypeDefinition
fn type_def_fields(
interner: &NodeInterner,
interner: &mut NodeInterner,
mut arguments: Vec<(Value, Location)>,
) -> IResult<Value> {
assert_eq!(arguments.len(), 1, "ICE: `generics` should only receive a single argument");
Expand All @@ -145,7 +144,9 @@ fn type_def_fields(

for (name, typ) in struct_def.get_fields_as_written() {
let name = make_quoted(vec![make_token(name)]);
let typ = Value::Code(Rc::new(type_to_tokens(&typ)?));
let id = interner.push_quoted_type(typ);
let typ = SpannedToken::new(Token::QuotedType(id), span);
let typ = Value::Code(Rc::new(Tokens(vec![typ])));
fields.push_back(Value::Tuple(vec![name, typ]));
}

Expand All @@ -155,22 +156,3 @@ fn type_def_fields(
])));
Ok(Value::Slice(fields, typ))
}

/// FIXME(https://github.com/noir-lang/noir/issues/5309): This code is temporary.
/// It will produce poor results for type variables and will result in incorrect
/// spans on the returned tokens.
fn type_to_tokens(typ: &Type) -> IResult<Tokens> {
let (mut tokens, mut errors) = Lexer::lex(&typ.to_string());

if let Some(last) = tokens.0.last() {
if matches!(last.token(), Token::EOF) {
tokens.0.pop();
}
}

if !errors.is_empty() {
let error = errors.swap_remove(0);
todo!("Got lexer error: {error}")
}
Ok(tokens)
}
1 change: 1 addition & 0 deletions compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,7 @@ impl<'a> Resolver<'a> {
Type::MutableReference(Box::new(self.resolve_type_inner(*element)))
}
Parenthesized(typ) => self.resolve_type_inner(*typ),
Resolved(id) => self.interner.get_quoted_type(id).clone(),
};

if let Type::Struct(_, _) = resolved_type {
Expand Down
17 changes: 16 additions & 1 deletion compiler/noirc_frontend/src/lexer/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ use acvm::{acir::AcirField, FieldElement};
use noirc_errors::{Position, Span, Spanned};
use std::{fmt, iter::Map, vec::IntoIter};

use crate::{lexer::errors::LexerErrorKind, node_interner::ExprId};
use crate::{
lexer::errors::LexerErrorKind,
node_interner::{ExprId, QuotedTypeId},
};

/// Represents a token in noir's grammar - a word, number,
/// or symbol that can be used in noir's syntax. This is the
Expand All @@ -24,6 +27,7 @@ pub enum BorrowedToken<'input> {
LineComment(&'input str, Option<DocStyle>),
BlockComment(&'input str, Option<DocStyle>),
Quote(&'input Tokens),
QuotedType(QuotedTypeId),
/// <
Less,
/// <=
Expand Down Expand Up @@ -125,6 +129,11 @@ pub enum Token {
BlockComment(String, Option<DocStyle>),
// A `quote { ... }` along with the tokens in its token stream.
Quote(Tokens),
/// A quoted type resulting from a `Type` object in noir code being
/// spliced into a macro's token stream. We preserve the original type
/// to avoid having to tokenize it, re-parse it, and re-resolve it which
/// may change the underlying type.
QuotedType(QuotedTypeId),
/// <
Less,
/// <=
Expand Down Expand Up @@ -223,6 +232,7 @@ pub fn token_to_borrowed_token(token: &Token) -> BorrowedToken<'_> {
Token::LineComment(ref s, _style) => BorrowedToken::LineComment(s, *_style),
Token::BlockComment(ref s, _style) => BorrowedToken::BlockComment(s, *_style),
Token::Quote(stream) => BorrowedToken::Quote(stream),
Token::QuotedType(id) => BorrowedToken::QuotedType(*id),
Token::IntType(ref i) => BorrowedToken::IntType(i.clone()),
Token::Less => BorrowedToken::Less,
Token::LessEqual => BorrowedToken::LessEqual,
Expand Down Expand Up @@ -343,6 +353,8 @@ impl fmt::Display for Token {
}
write!(f, "}}")
}
// Quoted types only have an ID so there is nothing to display
Token::QuotedType(_) => write!(f, "(type)"),
Token::IntType(ref i) => write!(f, "{i}"),
Token::Less => write!(f, "<"),
Token::LessEqual => write!(f, "<="),
Expand Down Expand Up @@ -394,6 +406,7 @@ pub enum TokenKind {
Keyword,
Attribute,
Quote,
QuotedType,
UnquoteMarker,
}

Expand All @@ -406,6 +419,7 @@ impl fmt::Display for TokenKind {
TokenKind::Keyword => write!(f, "keyword"),
TokenKind::Attribute => write!(f, "attribute"),
TokenKind::Quote => write!(f, "quote"),
TokenKind::QuotedType => write!(f, "quoted type"),
TokenKind::UnquoteMarker => write!(f, "macro result"),
}
}
Expand All @@ -424,6 +438,7 @@ impl Token {
Token::Attribute(_) => TokenKind::Attribute,
Token::UnquoteMarker(_) => TokenKind::UnquoteMarker,
Token::Quote(_) => TokenKind::Quote,
Token::QuotedType(_) => TokenKind::QuotedType,
tok => TokenKind::Token(tok.clone()),
}
}
Expand Down
18 changes: 18 additions & 0 deletions compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
use iter_extended::vecmap;
use noirc_arena::{Arena, Index};
use noirc_errors::{Location, Span, Spanned};
use petgraph::algo::tarjan_scc;

Check warning on line 10 in compiler/noirc_frontend/src/node_interner.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (petgraph)

Check warning on line 10 in compiler/noirc_frontend/src/node_interner.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (tarjan)
use petgraph::prelude::DiGraph;

Check warning on line 11 in compiler/noirc_frontend/src/node_interner.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (petgraph)
use petgraph::prelude::NodeIndex as PetGraphIndex;

use crate::ast::Ident;
Expand Down Expand Up @@ -176,6 +176,12 @@

/// Stores the [Location] of a [Type] reference
pub(crate) type_ref_locations: Vec<(Type, Location)>,

/// In Noir's metaprogramming, a noir type has the type `Type`. When these are spliced
/// into `quoted` expressions, we preserve the original type by assigning it a unique id
/// and creating a `Token::QuotedType(id)` from this id. We cannot create a token holding
/// the actual type since types do not implement Send or Sync.
quoted_types: noirc_arena::Arena<Type>,
}

/// A dependency in the dependency graph may be a type or a definition.
Expand Down Expand Up @@ -472,6 +478,9 @@
pub value: Option<comptime::Value>,
}

#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct QuotedTypeId(noirc_arena::Index);

impl Default for NodeInterner {
fn default() -> Self {
let mut interner = NodeInterner {
Expand Down Expand Up @@ -506,6 +515,7 @@
primitive_methods: HashMap::new(),
type_alias_ref: Vec::new(),
type_ref_locations: Vec::new(),
quoted_types: Default::default(),
};

// An empty block expression is used often, we add this into the `node` on startup
Expand Down Expand Up @@ -1735,6 +1745,14 @@

cycle
}

pub fn push_quoted_type(&mut self, typ: Type) -> QuotedTypeId {
QuotedTypeId(self.quoted_types.insert(typ))
}

pub fn get_quoted_type(&self, id: QuotedTypeId) -> &Type {
&self.quoted_types[id.0]
}
}

impl Methods {
Expand Down
14 changes: 13 additions & 1 deletion compiler/noirc_frontend/src/parser/parser/types.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use super::primitives::token_kind;
use super::{
expression_with_precedence, keyword, nothing, parenthesized, path, NoirParser, ParserError,
ParserErrorReason, Precedence,
Expand All @@ -6,7 +7,7 @@ use crate::ast::{Recoverable, UnresolvedType, UnresolvedTypeData, UnresolvedType
use crate::QuotedType;

use crate::parser::labels::ParsingRuleLabel;
use crate::token::{Keyword, Token};
use crate::token::{Keyword, Token, TokenKind};

use chumsky::prelude::*;
use noirc_errors::Span;
Expand All @@ -28,6 +29,7 @@ pub(super) fn parse_type_inner<'a>(
top_level_item_type(),
type_of_quoted_types(),
quoted_type(),
resolved_type(),
format_string_type(recursive_type_parser.clone()),
named_type(recursive_type_parser.clone()),
named_trait(recursive_type_parser.clone()),
Expand Down Expand Up @@ -105,6 +107,16 @@ fn quoted_type() -> impl NoirParser<UnresolvedType> {
.map_with_span(|_, span| UnresolvedTypeData::Quoted(QuotedType::Quoted).with_span(span))
}

/// This is the type of an already resolved type.
/// The only way this can appear in the token input is if an already resolved `Type` object
/// was spliced into a macro's token stream via the `$` operator.
fn resolved_type() -> impl NoirParser<UnresolvedType> {
token_kind(TokenKind::QuotedType).map_with_span(|token, span| match token {
Token::QuotedType(id) => UnresolvedTypeData::Resolved(id).with_span(span),
_ => unreachable!("token_kind(QuotedType) guarantees we parse a quoted type"),
})
}

pub(super) fn string_type() -> impl NoirParser<UnresolvedType> {
keyword(Keyword::String)
.ignore_then(type_expression().delimited_by(just(Token::Less), just(Token::Greater)))
Expand Down
4 changes: 4 additions & 0 deletions tooling/nargo_fmt/src/rewrite/typ.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ pub(crate) fn rewrite(visitor: &FmtVisitor, _shape: Shape, typ: UnresolvedType)

format!("fn{env}({args}) -> {return_type}")
}
UnresolvedTypeData::Resolved(_) => {
unreachable!("Unexpected macro expansion of a type in nargo fmt input")
}

UnresolvedTypeData::Unspecified => todo!(),
UnresolvedTypeData::FieldElement
| UnresolvedTypeData::Integer(_, _)
Expand Down
Loading