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

Make the AST public #77

Closed
ahupp opened this issue Jun 9, 2023 · 9 comments
Closed

Make the AST public #77

ahupp opened this issue Jun 9, 2023 · 9 comments

Comments

@ahupp
Copy link

ahupp commented Jun 9, 2023

Most of the internals of starlark-rust are private or pub(crate). For some work I'm doing I need to access and modify the AST, so it would be nice to have those accessible. The README says:

We do not aim for API stability between releases, preferring to iterate quickly and refine the API as much as possible. But we do follow SemVer

So in principle this should be fine. A good middle ground might be to move the AST modules to their own crate, then the existing starlark crate would not need to make breaking changes as often. Moving the AST to its own crate would also open the path to moving the LSP server to its own crate as well.

Thoughts?

@stepancheg
Copy link
Contributor

I was thinking of extracting AST into a separate crate to make compilation faster.

Moving LSP into a separate crate also makes sense because LSP pulls dependencies not needed by many users.

However, can you please describe your use case for AST?

@ahupp
Copy link
Author

ahupp commented Jun 10, 2023

can you please describe your use case for AST?

Two unrelated bits of work:

  • re-write (some) function calls to implicitly memoize their side effects on the filesystem when building a container image. In this case I need to know function entry and entry. AFAICT this isn't possible with the BeforeStmt tracing(?)
  • trace the control flow that produces each value, for a tool that will (hopefully!) do bidirectional updates between a UI and the code

@stepancheg
Copy link
Contributor

re-write (some) function calls to implicitly memoize their side effects

Shouldn't it be better done by the function implementation themselves? Perhaps, runtime wrappers of such functions?

Note AST is hard to rewrite correctly. Consider this example:

# read_file, native function

def make_container_image():
  alias = read_file
  read_file = 10
  alias("txt")

trace the control flow that produces each value

Note in tools like buck2 (but not only), we have higher level abstraction like "rules". I.e. intermediate values are not and memoized, but they are separate objects in the system. For example:

cxx_library(name = "hello")

cxx_binary(name = "world", deps = [":hello"])

Perhaps that's what you want. But I don't know your use case.

Let me think a bit, but I think we want to expose AST. I hesitate because with more public API, we break more code between releases.

@ahupp
Copy link
Author

ahupp commented Jun 13, 2023

Note in tools like buck2 (but not only), we have higher level abstraction like "rules".

Yup, I was at FB for a while and this is partially informed by experience with buck (haven't tried by buck2 yet), as well as my dislike of third-party2. I'm targeting something more like toast, but with a different interface.

with more public API, we break more code between releases.

Just make a new crate starlark-ast-do-not-use :)

@ndmitchell
Copy link
Contributor

Not a fan of extracting the LSP into a separate create - it complicates things, makes it much harder to do stuff, enforces public boundaries between areas. Is there any real benefit?

Given we already break a lot of stuff, my inclination is to just expose the AST and accept that it breaks between each release.

@stepancheg
Copy link
Contributor

@ahupp to avoid misunderstanding, I did not suggest using buck2 (although it also could be an option), but instead implement buck2-like setup.

@laurentlb
Copy link

fyi, https://github.com/bazelbuild/buildtools has a Go library that provides access to a Starlark AST (e.g. you can manipulate it and pretty print it). The interface is pretty stable and it is used by many Starlark tools. Maybe that will work for you use-case?

@stepancheg
Copy link
Contributor

I'm currently moving AST into a separate crate starlark_syntax (for faster incremental compilation; would take a couple of days to get into the repository, and more time to be released on crates). Feel free to use it if you want.

However, if you goal is codemod, I suggest using a library like libCST, since Starlark syntax is now officially a subset of Python.

@ahupp
Copy link
Author

ahupp commented Aug 31, 2023

That's an interesting point, I have some experience with writing syntax transformers with python's grammar in pegen which has similar benefits to libCST. Will take a look.

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

No branches or pull requests

4 participants