Record the four decisions, and settle div and mod - #1251
Conversation
Item 8 is closed rather than pending: div and mod keep returning the left operand's type, so real r = 7 div 2 compiles and is meant to. The branch in AttrExprType says so now, instead of looking like an oversight beside caseMathOperation, which collapses two literals to int precisely so real r = 1 + 1 is an error. Both tests which depend on it still pass. Item 7 records that the instantiation declares the module's type parameters only so a dispatch receiver has a name to resolve, and that they stay out of type inference - generic modules keep resolving theirs by matching the receiver type. The alternative, letting inference see them, collides with that and fails genericModuleInGenericClassGet; this is the smaller change at the price of the parameter meaning something narrower than it looks. What is left of the attempt is written down, including the widening the requirement lookup still needs. Item 23 records that Lua specialises only the paths needing a concrete type and leaves the object erased, because generated scripts should stay small. Items 6 and 13 both end there, and it removes the two-shapes-at-once arrangement which produced both field bugs in #1239. Instances for a family of types stay deferred, with why: it decides whether bounds stay a tool for new containers or become how the existing ones work.
A dispatch slot's name is composed from the segment after the last underscore of a method's mangled name, which is the declared name only when the declared name has no underscore in it. get_it contributes it, which is nobody's method, and the slot the call goes through is not the one the override was bound to. Found by auditing the junk-slot rule for what its name comparison does to unrelated methods, not by anyone hitting it, so it is worth pinning rather than leaving to be met by surprise. The test asserts the failure it currently produces and names what the fix has to be keyed on. Asking the declaration for the name is not that fix. declaredName already reads it off the trace, but two overloads share a declared name and mangle to Foo_bar and Foo_bar_1, so the segment after the last underscore is also what keeps their slots apart today. Pointing both composers at the declared name fixes the underscore case and collapses overloaded slots instead. What a method and its overrides share, and what tells two overloads apart, are different questions, and the mangled name is answering both at once by accident.
|
Added the underscore dispatch finding here, since it came out of auditing the junk-slot rule rather than out of anything on this branch. A dispatch slot's name is composed from the segment after the last underscore of a method's mangled name. That is the declared name only when the declared name has no underscore in it, so I tried the obvious fix and it does not hold, which is the more useful half. What a method and its overrides share, and what tells two overloads apart, are two different questions, and the mangled name is answering both at once by accident. The replacement is keyed on the declared signature, which is a real change rather than a rename, so it is backlog item 26 with that reasoning written down rather than something bundled in here. @codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d88a4ed9a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * the segment after the last underscore is also what currently keeps their slots apart. Backlog | ||
| * item 26 carries what the replacement has to be keyed on. | ||
| */ | ||
| @Test(expectedExceptions = Error.class, expectedExceptionsMessageRegExp = ".*Succeed function not called.*") |
There was a problem hiding this comment.
If a future dispatch change makes this call return any other incorrect value, the condition still skips testSuccess() and this expected-error test remains green, so it does not pin the described call-site/table-binding defect or distinguish it from a new dispatch regression. Capture the emitted Lua and assert both the slot used by h.get_it() and the Doubler/Holder assignments that currently mismatch.
AGENTS.md reference: AGENTS.md:L262-L270
Useful? React with 👍 / 👎.
|
Closing into #1253, which carries the pinned dispatch failure and the backlog entry with the evidence from both failed attempts at the fix. The div/mod decision and the other recorded decisions are in that branch's BACKLOG.md unchanged. |
1 participant
The owner answered the four open questions. This writes them into the backlog so they survive the session, and closes the one that needed no code.
Settled:
divandmodkeep returning the left operand's typeSo
real r = 7 div 2compiles, and it is meant to. Item 8 has been pending a yes or no since early in this work, with the behaviour pinned in both directions meanwhile.The branch in
AttrExprTypenow says so, rather than looking like an oversight besidecaseMathOperation— which collapses two integer literals to int precisely soreal r = 1 + 1is an error. The asymmetry is deliberate: the division is integer either way and the result is then widened.ExpressionTests.integerDivisionOfLiteralsIsStillAssignableToRealandOptimizerTests.realFormatting_consistent_fromIntOpsboth still pass, which is the point — the second opens with that assignment.Recorded: module bounds (item 7)
The instantiation declares the module's type parameters only so a dispatch receiver has a name to resolve, and they stay out of type inference. Generic modules keep resolving theirs by matching the receiver type.
The alternative — letting inference see the declared parameters — collides with that mechanism and fails
genericModuleInGenericClassGetwith "Cannot infer type for type parameter T". This is the smaller change, at the honest price of the parameter meaning something narrower than it appears to.The item now carries what the attempt reached, so it is resumable rather than rediscovered: the grammar change, the binding through
WurstTypeBoundTypeParam, the dispatch widening, and the one step left — the requirement lookup does not follow a binding to the underlying parameter's bounds.Recorded: the Lua erasure model (item 23)
Specialise only the paths which need a concrete type and leave the object erased. Generated scripts stay small, which is the reason; the cost is that it is more compiler work than not erasing at all.
The item spells out what that means for items 6 and 13 — the concrete type is threaded to the places which use it rather than to the object — and why it is worth doing rather than working around: two class shapes existing at once is what produced both field bugs in #1239.
Deferred, with the reason: instances for a family of types
It decides whether type class bounds stay a tool for new containers or become how
Table,HashMap,HashListandHashSetwork. Worth deciding when there is appetite for the language design, rather than alongside compiler work.Green:
ExpressionTests,OptimizerTests.