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

Add end offsets to AST and source map #1557

Conversation

iamdefinitelyahuman
Copy link
Contributor

@iamdefinitelyahuman iamdefinitelyahuman commented Aug 1, 2019

What I did

Added end offsets for the Vyper AST nodes and the source map.

How I did it

  • Add tokens to the python AST nodes using the asttokens library.
  • Use the tokens to add end_lineno and end_col_offset attributes to the final Vyper AST. (Attributes with these names are included in the standard ast library as of Python 3.8)
  • include these new attributes when generating the LLLnode objects, and add their values to the source map.

How to verify it

I've checked it manually but tests still need to be written, as well existing tests will need updating. I will do so if and when the concept is finalized / approved.

Comments / Thoughts

This PR is meant as a proof of concept. I assume it will need some discussion and work before it can be merged. The following questions come to mind:

  • Is it acceptable to introduce the asttokens library as a requirement? If not, I can instead take heavy inspiration from it and include the same functionality without requiring the import.
  • Is the output format for the source map offsets the correct approach? I've handled it as a 4 item tuple of (lineno, col_offset, end_lineno, end_col_offset). This is effectively a breaking change for any program expecting exactly a two item tuple. Other approaches could be:
    • a 2x2 tuple as [(lineno, col_offset), (end_lineno, end_col_offset)]
    • switch to using exact positions as (start, length) - more in line with solc output but less-so with python ASTs. Also a larger deviation from the current output and so a larger breaking change.

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@iamdefinitelyahuman
Copy link
Contributor Author

Based on the call I'm closing this. I will attempt to implement the same functionality without the asttokens import and open a new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant