Skip to content
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

feature request: add an extra field to AST Atom exprs that refer to the same "thing" #1537

Closed
fariedt opened this issue Aug 7, 2024 · 2 comments

Comments

@fariedt
Copy link

fariedt commented Aug 7, 2024

Is your feature request related to a problem? Please describe.

I am working on some scripts to analyze Clarity contracts. I use the AST generated by clarinet-sdk-wasm in JSON format instead of parsing the contracts; the AST contains almost everything I need1. One area where I've run into trouble is keeping track of which identifiers refer to which previously seen identifiers. Thanks to Clarity's shadowing rules, I believe this is only a problem in code that uses tuples with the same keys as other identifiers.

One example of this can be seen in the demo video for the check-checker, in the deposit function:

;; A user deposits money into the piggy bank.
(define-public (deposit (amount uint))
    (let ((balance (default-to 0 (get amount (map-get? accounts {holder: tx-sender})))))
        ;; A user should be allowed to specify any amount to deposit, and the
        ;; `stx-transfer?` will fail and rollback all operations if the sender
        ;; does not have enough STX, so it is safe to allow the unchecked data
        ;; here.
        ;; #[allow(unchecked_data)]
        (map-set accounts {holder: tx-sender} {amount: (+ balance (to-int amount))})
        (stx-transfer? amount tx-sender (as-contract tx-sender))
    )
)

The amount argument to get refers to the same thing as the amount key in the map-set call, and the amount function argument is the same as the amount in the map-set value and stx-transfer? call.

Describe the solution you'd like

I'd like to see an additional field added to Atom exprs (the SymbolicExpression struct?) in the AST which is the same for all identifiers that refer to the same thing. Clarity's VM code already does the work of matching identifiers when it parses and type-checks contracts.

If the field is called cid, I'd like it to act like this:

;; the color Atom has an id of 3 in the AST
(define-constant color 0x336699)

;; the color Atoms here have different ids in the AST but the same "cid"
;; value -- the id of color in the constant
(define-data-var list-colors (list 3 (buff 3)) (list color color color))

;; the color value against the color tuple key has a cid of 3
;; the color keys in both the type and value tuples have the same cid
(define-data-var site-colors
  { color: (buff 3), text-color: (buff 3) }
  { color: color, text-color: 0xff0000 })

;; color in the get call should have the same cid as its id in the site-colors tuple key
(define-read-only (get-text-color)
  (get color (var-get site-colors)))

Additional context

I don't know what cids for keywords like define-data-var or native functions should be.

Footnotes

  1. For return types of functions and types of constants, I use getContractsInterfaces().

@hugocaillard
Copy link
Collaborator

It looks like this kind of static analysis you're a looking for goes beyond the scope of parsing code into an ast.
I think that the AST isn't responsible for checking the logic and matching keywords like that

@hugocaillard
Copy link
Collaborator

@fariedt I'm closing for now following my previous comment.
Feel free to re-open if you have more feedback to share

@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in DevTools Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

2 participants