fix: Parse a statement as an expression (#6040)

# Description ## Problem\* Resolves #6036 ## Summary\* Grego was having an issue from some macros where some code was taken from a block and later put in the rhs of a `let` statement: `let _ = $former_block_statement`. Since this expression originated from a block statement though, it is a `Token::InternedStatement` and not a valid expression. I've updated the parser to accept interned statements in an expression position, and we just later validate that they're either `StatementKind::Expression` or `StatementKind::Semi` nodes. ## 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.
noir-lang · Sep 13, 2024 · ab203e4 · ab203e4
1 parent 7f2754e
commit ab203e4
Show file tree

Hide file tree

Showing 12 changed files with 128 additions and 15 deletions.
diff --git a/aztec_macros/src/utils/parse_utils.rs b/aztec_macros/src/utils/parse_utils.rs
@@ -299,6 +299,7 @@ fn empty_expression(expression: &mut Expression) {
         ExpressionKind::Quote(..)
         | ExpressionKind::Resolved(_)
         | ExpressionKind::Interned(_)
+        | ExpressionKind::InternedStatement(_)
         | ExpressionKind::Error => (),
     }
 }

diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs
@@ -7,7 +7,7 @@ use crate::ast::{
 };
 use crate::hir::def_collector::errors::DefCollectorErrorKind;
 use crate::macros_api::StructId;
-use crate::node_interner::{ExprId, InternedExpressionKind, QuotedTypeId};
+use crate::node_interner::{ExprId, InternedExpressionKind, InternedStatementKind, QuotedTypeId};
 use crate::token::{Attributes, FunctionAttribute, Token, Tokens};
 use crate::{Kind, Type};
 use acvm::{acir::AcirField, FieldElement};
@@ -48,6 +48,10 @@ pub enum ExpressionKind {
     // The actual ExpressionKind can be retrieved with a NodeInterner.
     Interned(InternedExpressionKind),
 
+    /// Interned statements are allowed to be parsed as expressions in case they resolve
+    /// to an StatementKind::Expression or StatementKind::Semi.
+    InternedStatement(InternedStatementKind),
+
     Error,
 }
 
@@ -617,6 +621,7 @@ impl Display for ExpressionKind {
                 write!(f, "quote {{ {} }}", tokens.join(" "))
             }
             AsTraitPath(path) => write!(f, "{path}"),
+            InternedStatement(_) => write!(f, "?InternedStatement"),
         }
     }
 }

diff --git a/compiler/noirc_frontend/src/ast/visitor.rs b/compiler/noirc_frontend/src/ast/visitor.rs
@@ -841,6 +841,7 @@ impl Expression {
             ExpressionKind::Quote(tokens) => visitor.visit_quote(tokens),
             ExpressionKind::Resolved(expr_id) => visitor.visit_resolved_expression(*expr_id),
             ExpressionKind::Interned(id) => visitor.visit_interned_expression(*id),
+            ExpressionKind::InternedStatement(id) => visitor.visit_interned_statement(*id),
             ExpressionKind::Error => visitor.visit_error_expression(),
         }
     }

diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs
@@ -25,9 +25,9 @@ use crate::{
     macros_api::{
         BlockExpression, CallExpression, CastExpression, Expression, ExpressionKind, HirLiteral,
         HirStatement, Ident, IndexExpression, Literal, MemberAccessExpression,
-        MethodCallExpression, PrefixExpression,
+        MethodCallExpression, PrefixExpression, StatementKind,
     },
-    node_interner::{DefinitionKind, ExprId, FuncId, TraitMethodId},
+    node_interner::{DefinitionKind, ExprId, FuncId, InternedStatementKind, TraitMethodId},
     token::Tokens,
     QuotedType, Shared, StructType, Type,
 };
@@ -67,6 +67,9 @@ impl<'context> Elaborator<'context> {
                 let expr = Expression::new(expr_kind.clone(), expr.span);
                 return self.elaborate_expression(expr);
             }
+            ExpressionKind::InternedStatement(id) => {
+                return self.elaborate_interned_statement_as_expr(id, expr.span);
+            }
             ExpressionKind::Error => (HirExpression::Error, Type::Error),
             ExpressionKind::Unquote(_) => {
                 self.push_err(ResolverError::UnquoteUsedOutsideQuote { span: expr.span });
@@ -80,6 +83,29 @@ impl<'context> Elaborator<'context> {
         (id, typ)
     }
 
+    fn elaborate_interned_statement_as_expr(
+        &mut self,
+        id: InternedStatementKind,
+        span: Span,
+    ) -> (ExprId, Type) {
+        match self.interner.get_statement_kind(id) {
+            StatementKind::Expression(expr) | StatementKind::Semi(expr) => {
+                self.elaborate_expression(expr.clone())
+            }
+            StatementKind::Interned(id) => self.elaborate_interned_statement_as_expr(*id, span),
+            StatementKind::Error => {
+                let expr = Expression::new(ExpressionKind::Error, span);
+                self.elaborate_expression(expr)
+            }
+            other => {
+                let statement = other.to_string();
+                self.push_err(ResolverError::InvalidInternedStatementInExpr { statement, span });
+                let expr = Expression::new(ExpressionKind::Error, span);
+                self.elaborate_expression(expr)
+            }
+        }
+    }
+
     pub(super) fn elaborate_block(&mut self, block: BlockExpression) -> (HirExpression, Type) {
         let (block, typ) = self.elaborate_block_expression(block);
         (HirExpression::Block(block), typ)

diff --git a/compiler/noirc_frontend/src/hir/comptime/value.rs b/compiler/noirc_frontend/src/hir/comptime/value.rs
@@ -24,7 +24,7 @@ use crate::{
         Expression, ExpressionKind, HirExpression, HirLiteral, Literal, NodeInterner, Path,
         StructId,
     },
-    node_interner::{ExprId, FuncId, StmtId, TraitId, TraitImplId},
+    node_interner::{ExprId, FuncId, InternedStatementKind, StmtId, TraitId, TraitImplId},
     parser::{self, NoirParser, TopLevelStatement},
     token::{SpannedToken, Token, Tokens},
     QuotedType, Shared, Type, TypeBindings,
@@ -454,6 +454,9 @@ impl Value {
             Value::Expr(ExprValue::Expression(expr)) => {
                 Token::InternedExpr(interner.push_expression_kind(expr))
             }
+            Value::Expr(ExprValue::Statement(StatementKind::Expression(expr))) => {
+                Token::InternedExpr(interner.push_expression_kind(expr.kind))
+            }
             Value::Expr(ExprValue::Statement(statement)) => {
                 Token::InternedStatement(interner.push_statement_kind(statement))
             }
@@ -872,9 +875,22 @@ fn remove_interned_in_expression_kind(
             remove_interned_in_expression_kind(interner, expr)
         }
         ExpressionKind::Error => expr,
+        ExpressionKind::InternedStatement(id) => remove_interned_in_statement_expr(interner, id),
     }
 }
 
+fn remove_interned_in_statement_expr(
+    interner: &NodeInterner,
+    id: InternedStatementKind,
+) -> ExpressionKind {
+    let expr = match interner.get_statement_kind(id).clone() {
+        StatementKind::Expression(expr) | StatementKind::Semi(expr) => expr.kind,
+        StatementKind::Interned(id) => remove_interned_in_statement_expr(interner, id),
+        _ => ExpressionKind::Error,
+    };
+    remove_interned_in_expression_kind(interner, expr)
+}
+
 fn remove_interned_in_literal(interner: &NodeInterner, literal: Literal) -> Literal {
     match literal {
         Literal::Array(array_literal) => {

diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs
@@ -130,6 +130,8 @@ pub enum ResolverError {
     ComptimeTypeInRuntimeCode { typ: String, span: Span },
     #[error("Comptime variable `{name}` cannot be mutated in a non-comptime context")]
     MutatingComptimeInNonComptimeContext { name: String, span: Span },
+    #[error("Failed to parse `{statement}` as an expression")]
+    InvalidInternedStatementInExpr { statement: String, span: Span },
 }
 
 impl ResolverError {
@@ -531,6 +533,13 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
                     *span,
                 )
             },
+            ResolverError::InvalidInternedStatementInExpr { statement, span } => {
+                Diagnostic::simple_error(
+                    format!("Failed to parse `{statement}` as an expression"),
+                    "The statement was used from a macro here".to_string(),
+                    *span,
+                )
+            },
         }
     }
 }
diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs
@@ -24,7 +24,10 @@
 //! be limited to cases like the above `fn` example where it is clear we shouldn't back out of the
 //! current parser to try alternative parsers in a `choice` expression.
 use self::path::as_trait_path;
-use self::primitives::{keyword, macro_quote_marker, mutable_reference, variable};
+use self::primitives::{
+    interned_statement, interned_statement_expr, keyword, macro_quote_marker, mutable_reference,
+    variable,
+};
 use self::types::{generic_type_args, maybe_comp_time};
 use attributes::{attributes, inner_attribute, validate_secondary_attributes};
 use doc_comments::{inner_doc_comments, outer_doc_comments};
@@ -512,15 +515,6 @@ where
     keyword(Keyword::Comptime).ignore_then(comptime_statement).map(StatementKind::Comptime)
 }
 
-pub(super) fn interned_statement() -> impl NoirParser<StatementKind> {
-    token_kind(TokenKind::InternedStatement).map(|token| match token {
-        Token::InternedStatement(id) => StatementKind::Interned(id),
-        _ => {
-            unreachable!("token_kind(InternedStatement) guarantees we parse an interned statement")
-        }
-    })
-}
-
 /// Comptime in an expression position only accepts entire blocks
 fn comptime_expr<'a, S>(statement: S) -> impl NoirParser<ExpressionKind> + 'a
 where
@@ -1158,6 +1152,7 @@ where
         as_trait_path(parse_type()).map(ExpressionKind::AsTraitPath),
         macro_quote_marker(),
         interned_expr(),
+        interned_statement_expr(),
     ))
     .map_with_span(Expression::new)
     .or(parenthesized(expr_parser.clone()).map_with_span(|sub_expr, span| {

diff --git a/compiler/noirc_frontend/src/parser/parser/primitives.rs b/compiler/noirc_frontend/src/parser/parser/primitives.rs
@@ -1,7 +1,7 @@
 use chumsky::prelude::*;
 
 use crate::ast::{ExpressionKind, GenericTypeArgs, Ident, PathSegment, UnaryOp};
-use crate::macros_api::UnresolvedType;
+use crate::macros_api::{StatementKind, UnresolvedType};
 use crate::parser::ParserErrorReason;
 use crate::{
     parser::{labels::ParsingRuleLabel, ExprParser, NoirParser, ParserError},
@@ -126,6 +126,26 @@ pub(super) fn interned_expr() -> impl NoirParser<ExpressionKind> {
     })
 }
 
+pub(super) fn interned_statement() -> impl NoirParser<StatementKind> {
+    token_kind(TokenKind::InternedStatement).map(|token| match token {
+        Token::InternedStatement(id) => StatementKind::Interned(id),
+        _ => {
+            unreachable!("token_kind(InternedStatement) guarantees we parse an interned statement")
+        }
+    })
+}
+
+// This rule is so that we can re-parse StatementKind::Expression and Semi in
+// an expression position (ignoring the semicolon) if needed.
+pub(super) fn interned_statement_expr() -> impl NoirParser<ExpressionKind> {
+    token_kind(TokenKind::InternedStatement).map(|token| match token {
+        Token::InternedStatement(id) => ExpressionKind::InternedStatement(id),
+        _ => {
+            unreachable!("token_kind(InternedStatement) guarantees we parse an interned statement")
+        }
+    })
+}
+
 #[cfg(test)]
 mod test {
     use crate::parser::parser::{

diff --git a/test_programs/compile_success_empty/comptime_parse_statement_as_expression/Nargo.toml b/test_programs/compile_success_empty/comptime_parse_statement_as_expression/Nargo.toml
@@ -0,0 +1,7 @@
+[package]
+name = "comptime_parse_statement_as_expression"
+type = "bin"
+authors = [""]
+compiler_version = ">=0.33.0"
+
+[dependencies]
diff --git a/test_programs/compile_success_empty/comptime_parse_statement_as_expression/src/main.nr b/test_programs/compile_success_empty/comptime_parse_statement_as_expression/src/main.nr
@@ -0,0 +1,27 @@
+fn main() {
+    comptime
+    {
+        let block = quote[{
+            1;
+            2;
+            3
+        }];
+        let statements = block.as_expr().unwrap().as_block().unwrap();
+        let last = statements.pop_back().1;
+
+        // `3` should  fit in an expression position even though it is
+        // originally a StatementKind::Expression
+        let regression1 = quote[{
+            let _ = $last;
+        }];
+        assert(regression1.as_expr().is_some());
+
+        // `1;` should  fit in an expression position even though it is
+        // originally a StatementKind::Semi
+        let first = statements.pop_front().0;
+        let regression2 = quote[{
+            let _ = $first;
+        }];
+        assert(regression2.as_expr().is_some());
+    }
+}
diff --git a/tooling/lsp/src/requests/inlay_hint.rs b/tooling/lsp/src/requests/inlay_hint.rs
@@ -542,6 +542,7 @@ fn get_expression_name(expression: &Expression) -> Option<String> {
         | ExpressionKind::Comptime(..)
         | ExpressionKind::Resolved(..)
         | ExpressionKind::Interned(..)
+        | ExpressionKind::InternedStatement(..)
         | ExpressionKind::Literal(..)
         | ExpressionKind::Unsafe(..)
         | ExpressionKind::Error => None,

diff --git a/tooling/nargo_fmt/src/rewrite/expr.rs b/tooling/nargo_fmt/src/rewrite/expr.rs
@@ -178,6 +178,11 @@ pub(crate) fn rewrite(
         ExpressionKind::Interned(_) => {
             unreachable!("ExpressionKind::Interned should only emitted by the comptime interpreter")
         }
+        ExpressionKind::InternedStatement(_) => {
+            unreachable!(
+                "ExpressionKind::InternedStatement should only emitted by the comptime interpreter"
+            )
+        }
         ExpressionKind::Unquote(expr) => {
             if matches!(&expr.kind, ExpressionKind::Variable(..)) {
                 format!("${expr}")