Skip to content

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Aug 17, 2023

To do the old version correctly, we would need to carefully track whether we end up using that parameter as a parameter of any apply_type call in the fieldtype computation of any of the fields, recursively. Since any of those might cause recursion in the constructor graph, resulting in trying to layout some concrete type before we have finished computing the rest of the fieldtypes. That seems unnecessarily difficult to do well, so instead we go back to just doing the trivial cases.

@vtjnash vtjnash requested a review from JeffBezanson August 17, 2023 20:39
To do the old version correctly, we would need to carefully track
whether we end up using that parameter as a parameter of any apply_type
call in the fieldtype computation of any of the fields, recursively.
Since any of those might cause recursion in the constructor graph,
resulting in trying to layout some concrete type before we have finished
computing the rest of the fieldtypes. That seems unnecessarily difficult
to do well, so instead we go back to just doing the trivial cases.
@brenhinkeller brenhinkeller added the types and dispatch Types, subtyping and method dispatch label Aug 19, 2023
@vtjnash vtjnash merged commit 1182003 into master Aug 21, 2023
@vtjnash vtjnash deleted the jn/layout-recur branch August 21, 2023 15:30
@KristofferC KristofferC added the backport 1.10 Change should be backported to the 1.10 release label Jun 5, 2025
KristofferC pushed a commit that referenced this pull request Jun 5, 2025
To do the old version correctly, we would need to carefully track
whether we end up using that parameter as a parameter of any apply_type
call in the fieldtype computation of any of the fields, recursively.
Since any of those might cause recursion in the constructor graph,
resulting in trying to layout some concrete type before we have finished
computing the rest of the fieldtypes. That seems unnecessarily difficult
to do well, so instead we go back to just doing the trivial cases.

(cherry picked from commit 1182003)
KristofferC pushed a commit that referenced this pull request Jun 5, 2025
To do the old version correctly, we would need to carefully track
whether we end up using that parameter as a parameter of any apply_type
call in the fieldtype computation of any of the fields, recursively.
Since any of those might cause recursion in the constructor graph,
resulting in trying to layout some concrete type before we have finished
computing the rest of the fieldtypes. That seems unnecessarily difficult
to do well, so instead we go back to just doing the trivial cases.

(cherry picked from commit 1182003)
KristofferC pushed a commit that referenced this pull request Jul 3, 2025
To do the old version correctly, we would need to carefully track
whether we end up using that parameter as a parameter of any apply_type
call in the fieldtype computation of any of the fields, recursively.
Since any of those might cause recursion in the constructor graph,
resulting in trying to layout some concrete type before we have finished
computing the rest of the fieldtypes. That seems unnecessarily difficult
to do well, so instead we go back to just doing the trivial cases.

(cherry picked from commit 1182003)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 1.10 Change should be backported to the 1.10 release types and dispatch Types, subtyping and method dispatch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants