Skip to content

Commit

Permalink
fix: Fix tokenization of unquoted types in macros (#5326)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves #5309

## Summary\*

Fixes the tokenization of types by converting them to a new `QuotedType`
token. We can get the original (resolved) type from this token so that
we don't need to re-resolve a type after it is inserted into a macro's
token stream. A QuotedType token holds onto an ID for the type rather
than the type itself since our `Type` type does not implement Send or
Sync, which is required through ParseError and UnresolvedTypeData.

## Additional Context

## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
jfecher authored Jun 25, 2024
1 parent b3a2c9c commit 6673c8b
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 26 deletions.
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 traits::*;
pub use type_alias::*;

use crate::{
node_interner::QuotedTypeId,
parser::{ParserError, ParserErrorReason},
token::IntType,
BinaryTypeOperator,
Expand Down Expand Up @@ -119,6 +120,10 @@ pub enum UnresolvedTypeData {
// The type of quoted code for 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 @@ impl std::fmt::Display for UnresolvedTypeData {
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 @@ impl<'a> Interpreter<'a> {
.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)
} else if let Some(foreign) = func_attrs.foreign() {
let item = format!("Comptime evaluation for foreign functions like {foreign}");
Err(InterpreterError::Unimplemented { item, location })
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 @@ -176,6 +176,12 @@ pub struct NodeInterner {

/// 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 struct GlobalInfo {
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 @@ impl Default for NodeInterner {
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 @@ impl NodeInterner {

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

0 comments on commit 6673c8b

Please sign in to comment.