-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Move JuliaSyntax + JuliaLowering into the main tree #59870
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: master
Are you sure you want to change the base?
Conversation
* A vector of `Slot`s is now created and passed into the `CodeInfo` creation pass so that code doesn't need access to the `Bindings` anymore. This is a better separation of data structures between passes. * Use K"Placeholder" for unused slots. * Fix small bug which made argument slurping broken.
Also fix a bug in linearization of `K"isdefined"`
Co-authored-by: spaette <[email protected]>
…#511) Julia's ecosystem (including Base.Docs and flisp lowering) assumes that strings within `struct` definitions are per-field docstrings, but the flisp parser doesn't handle these - they are only recognized when the struct itself has a docstring and are processed by the `@doc` macro recursing into the struct's internals. For example, the following doesn't result in any docs attached to `A`. ```julia struct A "x_docs" x "y_docs" y end ``` This change adds `K"doc"` node parsing to the insides of a struct, making the semantics clearer in the parser tree and making it possible to address this problems in the future within JuliaLowering. Also ensure that the `Expr` form is unaffected by this change.
…ng/JuliaSyntax.jl#506) * Don't assume that `SubString` has `pointer` and copy instead * Still assume `Substring{String}` has `pointer` * Test with `Test.GenericString`
…liaLang/JuliaSyntax.jl#500) * Remove the method `convert(::Type{String}, ::Kind)` This patch removes the method `convert(::Type{String}, ::Kind)` used for converting kinds to strings and replaces it with the already existing method of `Base.string`. There are two reason for this: i) the method causes invalidations when loading the package and ii) `convert` is called implicitly in e.g. constructors and should therefore typically only be defined between similar enough types. * Remove the method `Base.convert(::Type{Kind}, ::String)` This patch removes the method `Base.convert(::Type{Kind}, ::AbstractString)` and replaces it with a `Kind(::AbstractString)` constructor. The reason for this is that `convert` is called implicitly in e.g. constructors and should therefore typically only be defined between similar enough types.
Also introduce `K"code_info"` to distinguish the `CodeInfo`-like form with indexed statements from the more symbolic cross references that are used internally by lowering within `K"lambda"` prior to statement+SSA renumbering.
Still todo: * inner constructors * outer constructors * doc binding Also included here is `K"alias_binding"` - a more general replacement for the `outerref` used in flisp lowering. `alias_binding` allows one to allocate a binding early during desugaring and make this binding an alias for a given name. Bindings don't participate in scope resolution, so this allows us to bypass the usual scoping rules. For example, to refer to a global struct_name from an outer scope, but within an inner scope where the identifier struct_name is bound to a local variable. (We could also replace outerref by generating a new scope_layer and perhaps that would be simpler?)
This form where `K"lambda"` has four children [args, static_parameters, body, ret_var] feels more natural as it keeps AST pieces within the AST rather than as auxiliary attributes. These pieces do still need special treatment in scope resolution, but lambdas are already special there.
Avoid creating `::` expressions - just add these directly to the function argument name and type lists instead.
Perhaps this was used historically but it's now only used for method tables in method overlays.
As much as alias_binding is a neat idea, it seems like using a scope layer to distinguish the global vs local bindings might be good enough and allow us to remove the alias_binding concept. As a side effect, this may allow us to avoid needing support arbitrary bindings in some early lowering code.
Detangling this ball of string ... felt quite epic 😬😅 Here we take a different approach from the flisp code - we don't try to reproduce the function signature matching logic of `expand_function_def` to rewrite constructor signatures within the struct expansion code. Instead, we harness that existing logic by calling expand_function_def with custom rewrite functions for the inner part of the signature expression and the function body where `new()` occurs.
* Remove outterref - this has been removed upstream * Make expand_unionall_def its own function - this will be required shortly to match some changes upstream. * JuliaSyntax has removed the `convert` overload for `Kind` in the latest dev version
…args an error Here we introduce a `meta` attribute rather than - or perhaps in addition to - the `K"meta"` kind and use it to tag local variables which derived from function argument destructuring. We use this to make it an error to have duplicate destructured argument names. This is technically breaking, but probably only a good thing - without this users will silently have the intial duplicate argument names overwritten with the result of the last destructuring assignment. Also add tests for the various variable scope conflict errors: argument/local, static-parameter/local, local/global etc.
…wering These macros are a part of the language itself because they emit special syntax trees which is known to lowering. This is regardless of the fact that they don't have a surface syntax form. Where a `Base` form of these exists we add a method to that macro so it can be used as usual without needing to import from JuliaLowering.
I've chosen to attach the nospecialize metadata here as an attribute on the function argument names. This ensures it travels with the function arguments, without otherwise disturbing the AST. TODO: `@ nospecialise` within the function body is not done yet - it's not very natural in this scheme but I guess we should still be able to recognize it during scope analysis and turn it into a tombstone, moving the metadata onto the lambda's argument bindings.
Co-authored-by: Dilum Aluthge <[email protected]>
I'm totally fine with this, I just know very little about buildkite and JuliaSyntax is unique in the main repo (I think?) for being a package which should be tested against both the DEV version of julia and a matrix of historical versions. If we're ok with using github actions for the historical versions for now I'd like to do that so we can get this merged instead of me fighting a protracted battle against buildkite 😅 For testing against the DEV version of Julia, I want to integrate JuliaSyntax and JuliaLowering into |
|
Is it just JuliaSyntax that you want to test against old Julia versions? Or do you want to test both JuliaSyntax and JuliaLowering against old Julia versions? |
|
Should just be JuliaSyntax, since lowering is only aiming to be compatible with nightly for now |
|
Are newer versions of JuliaSyntax going to be installable on older Julia versions? |
|
Yes, as a package |
d228af6 to
df28c66
Compare
Needed a minor tweak, but looks like the buildbots are happy 👍 (except for some bad macOS configs in the test matrix) What else is needed for this PR to be merge-able? |
|
There are other CI changes I'd like us to make before merging. |
|
I'll make a PR against this PR, with the changes I'd like us to make before merging. |
|
I've attempted to fix the macOS CI issues. I didn't include the Currently JuliaLowering isn't hooked up to CI but I guess we could do that separately if people want to get this merged. It's not critical but I'd like to recreate the merges to clean up the history slightly and get in a few things which were already merged over in JuliaLowering. |
I would include it. Intel macOS is still a Tier 1 platform for Julia. |
I would probably prefer to get all the CI stuff finalized before merging this PR. |
But the |
Hooking JuliaLowering tests up to We could also use JuliaLowering's existing github actions setup in the meantime. @DilumAluthge do you prefer that or just going straight to |
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.
Discussed out-of-band, but briefly:
make testfor JL.make testfor JS on master.- GHA for JS on older Julia, because job matrix syntax is simple in GHA.
|
Oh, and ideally get an "approve" from someone on the non-CI aspects of this PR. cc: @Keno @JeffBezanson |
* Interpolation and type-stability improvements Should be a quick fix for #94. Also improve the interpolation algorithm: instead of starting with a copy of the AST and re-scanning the tree for interpolations with each call to `_interpolate_ast`, do one full unconditional pass over the initial tree that copies and interpolates. Also fixes interpolation into QuoteNode in expr compat mode (e.g. `@eval Base.$x`) --------- Co-authored-by: Claire Foster <[email protected]>
…109) Macros may pull apart an expression (eg, a module expression or the right hand side of a `.` expression) or quote that expression, and we should keep track of the scope where this originated. A particular example is the `@eval` macro. Consider ``` let name = :x @eval A.$name end ``` In this case the right hand side of `.` would normally be quoted (as a plain symbol) but in the case of `@eval` an extra `quote` is added around the expression to make the `name` variable valid unquoted code after quote expansion. In general, macros may pull apart or rearrange what's passed to them, so we can't make the assumption that normally-inert syntax passed to them should go without a scope layer. To fix this, this change adds a scope layer to all ASTs passed to macros. After macro expansion is done, we can then remove the layer from any AST we know is definitely inert to prevent it from interfering with future lowering passes over that quoted code. This helps but isn't a full solution - see #111 for further work.
For vendoring into Base we need to avoid absolute import paths as in `using JuliaLowering` and `using JuliaSyntax` in the test files as neither of these packages will be top level modules. Thus, replace all occurrences of these with relative import paths except for one central location (currently in util.jl) which can be easily adjusted.
There's been some interest in having the new Julia compiler frontend (JuliaSyntax + JuliaLowering) in the main Julia tree so that these are easier to work on together and so the new lowering code can co-evolve with changes to Core more easily.
Here's a simple sketch for moving both these libraries into the main tree as separate top level modules in the JuliaSyntax and JuliaLowering subdirectories. For git history, I've used
git-filter-repoto rewrite the history of both repositories into their respective subdirectories. At the same time some light rewriting was performed to avoid confusion for commit messages referring to issue numbers. For example, if a commit in the JuliaSyntax history refers to #256, that will be rewritten to the string JuliaLang/JuliaSyntax.jl#256. (Note for completeness that the history of these projects also includes the git history of Tokenize.jl which is the origin of the lexer.)There's a few questions / TODOs I'd like to consider before merging this:
How do we do CI of JuliaSyntax against old Julia versions?
JuliaSyntax currently supports Julia versions back to 1.0 (!!) Admittedly this may be excessive, but we should keep the JuliaSyntax registered in General working for at least some older Julia versions.
The problem is I know very little about how to set this up and I'd like advice or help :) @IanButterworth I can see you're active with both build kite and github actions infrastructure - I hoped you might have some thoughts or be able to point me in the right direction? Presumably we download pre-built versions from
julialang-s3.julialang.organd test the JuliaSyntax module against those in addition to the current dev version of Julia.Easing the archiving of JuliaLang/JuliaSyntax
There's enough open PRs on JuliaSyntax that it'd be nice to make migrating those to the main Julia repository easy. My rough plan is to filter all branches while running
git-filter-repoand push those filtered branches to JuliaLang/JuliaSyntax. Then PR authors should be able to grab the filtered version of their branch and apply it to the main Julia repo without issues. I haven't figured out the details of this yet but it should be done in onegit-filter-reporun to ensure consistency of version hashes.When this is done I'll also move c42f/JuliaLowering.jl into JuliaLang/JuliaLowering.jl and archive it so there's a more permanent home for the associated github issue and PR discussions.
What should these modules be called?
I hesitate to bring this up because it might become a distraction. But if we want to rename either of these modules it makes sense to do it now while we're moving git histories around.
Originally,
JuliaSyntaxwas named that way because there was a very old and obsoleteJuliaParseralready taking the name, and the prefix "Julia" was used for clarity given that it was going into the General registry. (Also, the parser work was started as an experimental side project and taking a canonical name seemed rather too bold 😅) If we want to claim a more canonical name at this point we might consider renaming it toParser. (Of course we could takeJuliaParseras a name, but that seems marginal enough that we may as well stick with the existing name.)JuliaLoweringwas named with the same convention but if we change the JuliaSyntax name to just Parser we might also consider renaming JuliaLowering to something likeLoweringorCodeLowering.CompilerFrontendis also a tempting name but not including the parser in the "compiler frontend" would be a bit weird.