essential-contributions / pint

Pint, the constraint-based programming language for declarative blockchains
Apache License 2.0
17 stars 3 forks source link

fix: handle macro declared vars #772

Closed freesig closed 3 months ago

freesig commented 3 months ago

If you declare a var inside a macro it gets a name like ::anon@0::v5. This handles that by also replacing @ and non-prefix :: with _.

Maybe don't merge until @mitchmindtree has checked this

freesig commented 3 months ago

fixes #771

freesig commented 3 months ago

Actually this doesn't handle pub vars declared in macros. Fixing this now Fixed this

freesig commented 3 months ago

I'm kind of surprised we can have macros that expand to var/pub-var declarations, but looks like this PR just helps support existing stuff so :shipit:

If we couldn't do this then it greatly reduces the ability to reuse code with macros. The vars/ pub vars are still scoped to the macro so they can't be used outside the macro.

mitchmindtree commented 3 months ago

I guess it just feels a bit odd that we abstract them away in a macro in pint, but then they're revealed on the application side in ABI-gen anyways.

My intuition is that like state, var and pub var declarations should be explicit as they're all the inputs to the predicate. Having them hidden in macros kind of feels like hiding function arguments :sweat_smile:

I'm struggling to think of cases where you wouldn't want to declare a var/pub var with some type, and then pass the var ident as an input to the macro instead to do whatever other constraint expansion etc is necessary :thinking: Maybe the use-case will become clearer to me once I actually start hacking on some application stuff.

freesig commented 3 months ago

I'm struggling to think of cases where you wouldn't want to declare a var/pub var with some type, and then pass the var ident as an input to the macro instead to do whatever other constraint expansion etc is necessary 🤔 Maybe the use-case will become clearer to me once I actually start hacking on some application stuff.

Basically something like this: https://github.com/essential-contributions/essential-integration/blob/d226ba333d1a27488589f298181a4deb84fe9961/apps/token/pint/signed/src/contract.pnt#L109

freesig commented 3 months ago

I guess if we decide to not allow declaring vars and pub vars in macros then I could not merge this PR and just change the integration apps to not do that?

@mitchmindtree another thing that this kind of fixed but might not actually be a problem is that currently if you have a name like ::foo::bar it becomes foo not foo_bar. I'm not sure if it's possible to declare a var or pub var in another file but if it is this would be a problem.

freesig commented 3 months ago

I might just merge this as it's blocking work on the integration repo. If we decide to make it not possible to declare var or pub var in macros then we can revert this change later.

mitchmindtree commented 3 months ago

I'm not sure if it's possible to declare a var or pub var in another file

I don't think it is, and I suspect we don't want to allow this. It might be odd to declare an input for a predicate somewhere outside of the predicate itself? Again, this might just be a self-imposed mental block I have by imagining vars and pub vars as function inputs for predicates, but it's the closest mental model I can think of. I feel like as someone reading the code, you want to be able to read all the inputs in one place, but this might just be personal preference.

I'll open a separate issue to discuss decision variable / transient data decls in macros :+1: