-
Notifications
You must be signed in to change notification settings - Fork 8
283 keep order of chain specific outputs #285
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
base: main
Are you sure you want to change the base?
Changes from 5 commits
769d503
967e013
ce63297
17f92b5
3871ac7
557894b
ca09b3f
d33d5cf
e4f8bed
ffe65c9
406cd93
0070efd
91d3622
06a445b
16457b4
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 |
|---|---|---|
|
|
@@ -586,6 +586,7 @@ pub struct CardanoPublishBlock { | |
| pub name: Option<Identifier>, | ||
| pub fields: Vec<CardanoPublishBlockField>, | ||
| pub span: Span, | ||
| pub declared_index: Option<usize>, | ||
| } | ||
|
|
||
| impl CardanoPublishBlock { | ||
|
|
@@ -663,7 +664,12 @@ impl AstNode for CardanoPublishBlock { | |
| .map(|x| CardanoPublishBlockField::parse(x)) | ||
| .collect::<Result<Vec<_>, _>>()?; | ||
|
|
||
| Ok(CardanoPublishBlock { name, fields, span }) | ||
| Ok(CardanoPublishBlock { | ||
| name, | ||
| fields, | ||
| span, | ||
| declared_index: None, | ||
| }) | ||
| } | ||
|
|
||
| fn span(&self) -> &Span { | ||
|
|
@@ -727,12 +733,21 @@ impl IntoLower for CardanoPublishBlock { | |
| &self, | ||
| ctx: &crate::lowering::Context, | ||
| ) -> Result<Self::Output, crate::lowering::Error> { | ||
| let data = self | ||
| let mut data: HashMap<String, ir::Expression> = self | ||
| .fields | ||
| .iter() | ||
| .map(|x| x.into_lower(ctx)) | ||
| .collect::<Result<_, _>>()?; | ||
|
|
||
| data.insert( | ||
| "declared_index".to_string(), | ||
| ir::Expression::Number( | ||
| self.declared_index | ||
| .map(|x| x as i128) | ||
| .expect("Publish block must have a declaration index"), | ||
| ), | ||
| ); | ||
|
|
||
|
Contributor
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 lowering phase should not be used for enforcing invariants. If the user should be aware of the rule, then the analyze phase should check the rule. The assertion of the rule should be applied when needed. In this case, the compiler should be then one erroring when the value is missing. |
||
| Ok(ir::AdHocDirective { | ||
| name: "cardano_publish".to_string(), | ||
| data, | ||
|
|
@@ -938,6 +953,7 @@ mod tests { | |
| ))), | ||
| ], | ||
| span: Span::DUMMY, | ||
| declared_index: None, | ||
| } | ||
| ); | ||
|
|
||
|
|
@@ -967,6 +983,7 @@ mod tests { | |
| ))), | ||
| ], | ||
| span: Span::DUMMY, | ||
| declared_index: None, | ||
| } | ||
| ); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -697,12 +697,14 @@ impl IntoLower for ast::OutputBlock { | |
| let address = self.find("to").into_lower(ctx)?.unwrap_or_default(); | ||
| let datum = self.find("datum").into_lower(ctx)?.unwrap_or_default(); | ||
| let amount = self.find("amount").into_lower(ctx)?.unwrap_or_default(); | ||
| let declared_ix = self.declared_index.unwrap(); | ||
|
|
||
| Ok(ir::Output { | ||
| address, | ||
| datum, | ||
| amount, | ||
| optional: self.optional, | ||
| declared_index: ir::Expression::Number(declared_ix as i128), | ||
|
Contributor
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. why do we use an explicit expression type? |
||
| }) | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -208,16 +208,32 @@ impl AstNode for TxDef { | |
| let mut signers = None; | ||
| let mut metadata = None; | ||
|
|
||
| let mut declared_index: usize = 0; | ||
|
|
||
| for item in inner { | ||
| match item.as_rule() { | ||
| Rule::locals_block => locals = Some(LocalsBlock::parse(item)?), | ||
| Rule::reference_block => references.push(ReferenceBlock::parse(item)?), | ||
| Rule::input_block => inputs.push(InputBlock::parse(item)?), | ||
| Rule::output_block => outputs.push(OutputBlock::parse(item)?), | ||
| Rule::output_block => { | ||
| let mut ob = OutputBlock::parse(item)?; | ||
| ob.declared_index = Some(declared_index); | ||
| declared_index += 1; | ||
| outputs.push(ob); | ||
|
Contributor
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. this shouldn't be here. We're coupling the rules of a specifc type of AST in the parsing logic of a totally unrelated block. On top of that, the AST is not in charge of defining the indexes of the outputs. The AST describes the intent of the user that typed the code. The index expression should remain as an optional expression all the way down until the compiler is forced to add a value. |
||
| } | ||
| Rule::validity_block => validity = Some(ValidityBlock::parse(item)?), | ||
| Rule::mint_block => mints.push(MintBlock::parse(item)?), | ||
| Rule::burn_block => burns.push(MintBlock::parse(item)?), | ||
| Rule::chain_specific_block => adhoc.push(ChainSpecificBlock::parse(item)?), | ||
| Rule::chain_specific_block => { | ||
| let mut csb = ChainSpecificBlock::parse(item)?; | ||
| let ChainSpecificBlock::Cardano(cardano_block) = &mut csb; | ||
| if let crate::cardano::CardanoBlock::Publish(pb) = cardano_block { | ||
| pb.declared_index = Some(declared_index); | ||
| declared_index += 1; | ||
| } | ||
|
|
||
| adhoc.push(csb); | ||
|
Contributor
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. same as previous comment. |
||
| } | ||
| Rule::collateral_block => collateral.push(CollateralBlock::parse(item)?), | ||
| Rule::signers_block => signers = Some(SignersBlock::parse(item)?), | ||
| Rule::metadata_block => metadata = Some(MetadataBlock::parse(item)?), | ||
|
|
@@ -617,6 +633,7 @@ impl AstNode for OutputBlock { | |
| optional, | ||
| fields, | ||
| span, | ||
| declared_index: None, | ||
| }) | ||
| } | ||
|
|
||
|
|
@@ -2464,6 +2481,7 @@ mod tests { | |
| }))), | ||
| ], | ||
| span: Span::DUMMY, | ||
| declared_index: None, | ||
| } | ||
| ); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -222,7 +222,8 @@ | |
| "dummy": false, | ||
| "start": 337, | ||
| "end": 412 | ||
| } | ||
| }, | ||
| "declared_index": 0 | ||
| } | ||
| ], | ||
| "validity": null, | ||
|
|
||
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.
why
declared_indexinstead ofindex?is there a conflicting index which is not declared?
if not, rename it to just
index