-
Notifications
You must be signed in to change notification settings - Fork 326
feat(assembly): add trace for generating read-only events
#3478
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
Changes from 2 commits
6d1073f
220f6e2
23fb57b
9615968
044144e
a3bfa28
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -337,6 +337,12 @@ impl PrettyPrint for Instruction { | |
| Self::Emit => const_text("emit"), | ||
| Self::EmitImm(value) => inst_with_felt_imm("emit", value), | ||
|
|
||
| // ----- traces (read-only events) ---------------------------------------------------- | ||
| Self::Trace => const_text("trace"), | ||
| // Printing `TraceImm` such that it is consistent with `EmitImm`, even though this | ||
| // does not round-trip. `trace.<FELT_IMM>` is invalid. | ||
| Self::TraceImm(value) => inst_with_felt_imm("trace", value), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Formatting a parsed I reproduced this with Could this print a valid equivalent such as
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The idea of printing as Should we open an issue to change the printing of
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| // Handled by the early return for !has_textual_representation() | ||
| Self::DebugVar(_) => unreachable!(), | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -377,6 +377,10 @@ static PRIMITIVE_SPECS: &[PrimitiveSpec] = &[ | |
| spelling: "emit", | ||
| build: || Instruction::Emit, | ||
| }, | ||
| PrimitiveSpec { | ||
| spelling: "trace", | ||
| build: || Instruction::Trace, | ||
| }, | ||
| PrimitiveSpec { | ||
| spelling: "eval_circuit", | ||
| build: || Instruction::EvalCircuit, | ||
|
|
@@ -1208,6 +1212,7 @@ fn lower_extended_instruction( | |
| }, | ||
|
|
||
| ExtendedInstructionKind::Emit => lower_emit_instruction(context, span, &tokens), | ||
| ExtendedInstructionKind::Trace => lower_trace_instruction(context, span, &tokens), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: THe instruction spec could carry the immediate constructor and use one lowering helper keyed by
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Not sure if I understand correctly, but Wouldn't that require changing For now I removed the duplication by lowering both through a new shared function: |
||
| ExtendedInstructionKind::ErrorCode(build) => { | ||
| lower_error_code_instruction(context, span, &tokens, spec.keyword, build) | ||
| }, | ||
|
|
@@ -1224,6 +1229,7 @@ enum ExtendedInstructionKind { | |
| Push, | ||
| Invocation(fn(ast::InvocationTarget) -> Instruction), | ||
| Emit, | ||
| Trace, | ||
| ErrorCode(fn(ast::ErrorMsg) -> Instruction), | ||
| } | ||
|
|
||
|
|
@@ -1252,6 +1258,10 @@ static EXTENDED_INSTRUCTION_SPECS: &[ExtendedInstructionSpec] = &[ | |
| keyword: "emit", | ||
| kind: ExtendedInstructionKind::Emit, | ||
| }, | ||
| ExtendedInstructionSpec { | ||
| keyword: "trace", | ||
| kind: ExtendedInstructionKind::Trace, | ||
| }, | ||
| ExtendedInstructionSpec { | ||
| keyword: "assert", | ||
| kind: ExtendedInstructionKind::ErrorCode(Instruction::AssertWithError), | ||
|
|
@@ -1383,6 +1393,46 @@ fn lower_emit_instruction( | |
| } | ||
| } | ||
|
|
||
| /// Lowers `trace.<const>` and `trace.event("name")`. | ||
| fn lower_trace_instruction( | ||
| context: &mut LoweringContext<'_>, | ||
| instruction_span: SourceSpan, | ||
| tokens: &[SyntaxToken], | ||
| ) -> Result<Option<Vec<ast::Op>>, ParsingError> { | ||
| if tokens.len() < 3 | ||
| || tokens[0].kind() != SyntaxKind::Ident | ||
| || tokens[0].text() != "trace" | ||
| || tokens[1].kind() != SyntaxKind::Dot | ||
| { | ||
| return Ok(None); | ||
| } | ||
|
|
||
| match &tokens[2..] { | ||
| [name] if name.kind() == SyntaxKind::Ident && name.text() != "event" => { | ||
| let name = context.lower_constant_ident_token(name)?; | ||
| Ok(Some(vec![inst_op( | ||
| instruction_span, | ||
| Instruction::TraceImm(Immediate::Constant(name)), | ||
| )])) | ||
| }, | ||
| [event, lparen, string, rparen] | ||
| if event.kind() == SyntaxKind::Ident | ||
| && event.text() == "event" | ||
| && lparen.kind() == SyntaxKind::LParen | ||
| && matches!(string.kind(), SyntaxKind::QuotedString | SyntaxKind::QuotedIdent) | ||
| && rparen.kind() == SyntaxKind::RParen => | ||
| { | ||
| let value = unquote_string_token(string, context.parse().span_for_token(string))?; | ||
| let event_id = EventId::from_name(value.as_ref()).as_felt(); | ||
| Ok(Some(vec![inst_op( | ||
| instruction_span, | ||
| Instruction::TraceImm(Immediate::Value(Span::new(instruction_span, event_id))), | ||
| )])) | ||
| }, | ||
| _ => Ok(None), | ||
| } | ||
| } | ||
|
|
||
| /// Lowers `.err=` forms for assertion-like instructions. | ||
| fn lower_error_code_instruction( | ||
| context: &mut LoweringContext<'_>, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also adds
TraceandTraceImmto the public exhaustivemiden_assembly_syntax::ast::Instructionenum, so downstream exhaustive matches stop compiling.cargo semver-checks check-release -p miden-assembly-syntax --baseline-version 0.29.0 --release-type minorreportsenum_variant_addedfor both variants.Could we mark this entry
[BREAKING]?