-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Very WIP: Mutually recursive types #32581
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When modifying Julia's builtin types, it's really easy to make mistakes here. Now the compiler will check for you.
I've started over on this. Some changes:
The method definition deferment behavior is still there, which @JeffBezanson was suspicious of, so that may be something that needs to be resolved soon. |
Keno
added a commit
that referenced
this pull request
Jul 23, 2019
This is an alternative to #32581, that is easier to implement, but more restrictivive. Here, the forward declaration must contain everything except for the field types: ``` incomplete type Foo{Bar} <: Baz; end ``` with it being an error to deviate from this specification when completing the type ``` struct Foo <: Baz; end # Error: Missing type parameter struct Foo{A, B} <: Baz; end # Error: Too many type parameters struct Foo{Qux} <: Baz; end # Error: Name of type parameter doesn't match struct Foo{Bar<:Int64} <: Baz; end # Error: Bounds of type parameter don't match struct Foo{Bar}; end; # Error supertype dosesn't match ``` This is easier because this way we have enough information for subtyping and type application and therefor do not need to delay this until the entire dependency graph is complete as we did in #32581. Of course this also means that we don't get the union feature that was requested in #269: ``` inomplete type U; end struct A; x::U; end struct B; x::U; end U = Union{A, B}; #ERROR ``` However, it could of course be emulated by wrapping the union type: ``` struct U data::Union{A, B} end ``` However, given the simplicity of this change and the difficulty of #32581, this seems like the way to go for the moment. We may want to revisit all this if we ever want to do computed field types, which will encounter similar challenges as #32581. Fixes #269.
Keno
added a commit
that referenced
this pull request
Jul 24, 2019
This is an alternative to #32581, that is easier to implement, but more restrictivive. Here, the forward declaration must contain everything except for the field types: ``` incomplete type Foo{Bar} <: Baz; end ``` with it being an error to deviate from this specification when completing the type ``` struct Foo <: Baz; end # Error: Missing type parameter struct Foo{A, B} <: Baz; end # Error: Too many type parameters struct Foo{Qux} <: Baz; end # Error: Name of type parameter doesn't match struct Foo{Bar<:Int64} <: Baz; end # Error: Bounds of type parameter don't match struct Foo{Bar}; end; # Error supertype dosesn't match ``` This is easier because this way we have enough information for subtyping and type application and therefor do not need to delay this until the entire dependency graph is complete as we did in #32581. Of course this also means that we don't get the union feature that was requested in #269: ``` inomplete type U; end struct A; x::U; end struct B; x::U; end U = Union{A, B}; #ERROR ``` However, it could of course be emulated by wrapping the union type: ``` struct U data::Union{A, B} end ``` However, given the simplicity of this change and the difficulty of #32581, this seems like the way to go for the moment. We may want to revisit all this if we ever want to do computed field types, which will encounter similar challenges as #32581. Fixes #269.
Keno
added a commit
that referenced
this pull request
Jul 26, 2019
This is an alternative to #32581, that is easier to implement, but more restrictivive. Here, the forward declaration must contain everything except for the field types: ``` incomplete type Foo{Bar} <: Baz; end ``` with it being an error to deviate from this specification when completing the type ``` struct Foo <: Baz; end # Error: Missing type parameter struct Foo{A, B} <: Baz; end # Error: Too many type parameters struct Foo{Qux} <: Baz; end # Error: Name of type parameter doesn't match struct Foo{Bar<:Int64} <: Baz; end # Error: Bounds of type parameter don't match struct Foo{Bar}; end; # Error supertype dosesn't match ``` This is easier because this way we have enough information for subtyping and type application and therefor do not need to delay this until the entire dependency graph is complete as we did in #32581. Of course this also means that we don't get the union feature that was requested in #269: ``` inomplete type U; end struct A; x::U; end struct B; x::U; end U = Union{A, B}; #ERROR ``` However, it could of course be emulated by wrapping the union type: ``` struct U data::Union{A, B} end ``` However, given the simplicity of this change and the difficulty of #32581, this seems like the way to go for the moment. We may want to revisit all this if we ever want to do computed field types, which will encounter similar challenges as #32581. Fixes #269.
Keno
added a commit
that referenced
this pull request
Jul 26, 2019
This is an alternative to #32581, that is easier to implement, but more restrictivive. Here, the forward declaration must contain everything except for the field types: ``` incomplete type Foo{Bar} <: Baz; end ``` with it being an error to deviate from this specification when completing the type ``` struct Foo <: Baz; end # Error: Missing type parameter struct Foo{A, B} <: Baz; end # Error: Too many type parameters struct Foo{Qux} <: Baz; end # Error: Name of type parameter doesn't match struct Foo{Bar<:Int64} <: Baz; end # Error: Bounds of type parameter don't match struct Foo{Bar}; end; # Error supertype dosesn't match ``` This is easier because this way we have enough information for subtyping and type application and therefor do not need to delay this until the entire dependency graph is complete as we did in #32581. Of course this also means that we don't get the union feature that was requested in #269: ``` inomplete type U; end struct A; x::U; end struct B; x::U; end U = Union{A, B}; #ERROR ``` However, it could of course be emulated by wrapping the union type: ``` struct U data::Union{A, B} end ``` However, given the simplicity of this change and the difficulty of #32581, this seems like the way to go for the moment. We may want to revisit all this if we ever want to do computed field types, which will encounter similar challenges as #32581. Fixes #269.
Abandoning in favor of #32658 |
Keno
added a commit
that referenced
this pull request
Aug 2, 2019
This is an alternative to #32581, that is easier to implement, but more restrictivive. Here, the forward declaration must contain everything except for the field types: ``` incomplete type Foo{Bar} <: Baz; end ``` with it being an error to deviate from this specification when completing the type ``` struct Foo <: Baz; end # Error: Missing type parameter struct Foo{A, B} <: Baz; end # Error: Too many type parameters struct Foo{Qux} <: Baz; end # Error: Name of type parameter doesn't match struct Foo{Bar<:Int64} <: Baz; end # Error: Bounds of type parameter don't match struct Foo{Bar}; end; # Error supertype dosesn't match ``` This is easier because this way we have enough information for subtyping and type application and therefor do not need to delay this until the entire dependency graph is complete as we did in #32581. Of course this also means that we don't get the union feature that was requested in #269: ``` inomplete type U; end struct A; x::U; end struct B; x::U; end U = Union{A, B}; #ERROR ``` However, it could of course be emulated by wrapping the union type: ``` struct U data::Union{A, B} end ``` However, given the simplicity of this change and the difficulty of #32581, this seems like the way to go for the moment. We may want to revisit all this if we ever want to do computed field types, which will encounter similar challenges as #32581. Fixes #269.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a very WIP start at #269. In fact it's so WIP that it doesn't build and is probably not worth looking at the code in its current form (http://wiki.c2.com/?PlanToThrowOneAway).
Here's how things currently work on this PR:
this declares two mutually recursive structs Foo and Bar, with an explicit forward declaration for Bar. At the moment this is being accomplished by having
Bar
be an incomplete DataType that then gets updated in place when it is completed. However, I suspect this will need to be changed to a more general mechanism with pointers being replaced after the fact, since in general we don't necessarily want to assume thatBar
will be a DataType (e.g. as in theUnion
example in #269).We also keep track of the completeness or non-completness of any given data type:
(0x0 indicates complete). All types in a strongly connected component of the dependency graph are finalized together when the last dependency of the SCC is resolved.
Additionally, incomplete types are invalid in subtyping, so method definitions are delayed until
the relevant types are completed:
I'm planning to require all types to be completed by the end of their defining module, though I haven't implemented that yet, i.e. we'd have:
Additionally, there's of course the question of automatically inserting forward definitions. I think that's more of a UX tradeoff than a technical one. I see basically three possibilities.
would be semantically equivalent to
though of course this would be done in the runtime, not during lowering, since we don't know what identifiers have been defined already during lowering.
This'd probably be the most convenient, but I'm slightly worried about causing user confusion as to when the RHS of a struct definition is evaluated, particularly wrt evaluated definitions:
I'm also a bit worried about user confusion around typos, e.g.
That could probably be addressed with appropriate error messages and warnings, but I think it's a not-insignificant concern.
This is basically what was proposed in #269. Note that I don't actually think this lets us avoid any technical difficulties over the other proposal, but does of course have a more limited scope for the automatic forward declarations so avoids some of the UX difficulties of the previous option. To recap, the proposal would be that:
would lower to
I think the biggest problem with this is that it makes the extent of the lowering scope semantically significant at top level, far more than anything else in the language. I.e. you could no longer safely paste the inside of that
begin/end
block into the REPL (particularly if we allow it without begin/end in an enclosing module or at the file level).Simple, but perhaps not satisfying.
Anyway, comments welcome. cc @StefanKarpinski @JeffBezanson