frontend: internal representation of method calls skips separate bind step#447
Conversation
… step
Previously, method calls were represented using separate bind and call steps.
There's something elegant to treating complex calls like the below as
multiple distinct steps.
subject.method<TypeActual>(arg0, arg1)
┣━━━━━┛ ┃ ┃ ┃
┃SUBJECT ┃ ┃ ┃
┣━━━━━━━━━━━━┛ ┃ ┃
┃ BOUND METHOD ┃ ┃
┣━━━━━━━━━━━━━━━━━━━━━━━━┛ ┃
┃ BOUND AND PARAMETERIZED METHOD ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
COMPLETE METHOD CALL
It was convenient to type each part separately.
- The subject has type `MyClass<Bar>`
- Imagine, the method, pre-binding to a subject, has type
`<U>(MyClass<T>) -> (U, U) -> Foo<T, U>`,
so when bound to `MyClass<Bar>` the *T* binds to *Bar* and the overall type
for the bound method has type `<U>(U, U) -> Foo<Bar, U>`
- Since there are explicit type parameters, the bound and parameterized
method has type `(TypeActual, TypeActual) -> Foo<Bar, TypeActual>`
- Then the method call has type `Foo<Bar, TypeActual>`.
That only works if you can represent calls to overloaded methods as
one type, an intersection (`&`) type of function types. Note also
that the type of the bound method is a function type that returns a
function type. There's currying going on. Code reasoning about this
has to repeatedly pack and unpack intersection types and curry and
uncurry type info.
Translating intersection types is hard; most typed languages
(TypeScript being an exception) do not allow user's to explicitly
specify intersection types, though many use them under the hood.
Also, reasoning about whole calls is more common than reasoning about the
parts above (again, outside of the Typer), so many parts of the frontend and
the TmpL translator were complicated by the need to look through calls
so that the could reassemble full argument lists (subject and args)
across two calls.
Ongoing changes to the internal representation of types and to type
solving made it easier to sequester overload handling to a small
portion of the *Typer* and *TypeSolver*, obviating the need for
intersection types, and the need for union types (previously used for
nullable type representation and unions of success and failure types
in result types).
This commit reworks *DotHelper*; instead of *BindMemberAccessor*
specifying bind semantics, now *CallMemberAccessor* specifies full
call semantics.
There is a lot of cleanup and reworking of *Typer* internally to
type the new representation, and cleanup of existing code that
produces method calls to produce the newer, flatter calls.
A lot of this reworking of the typer prefigures work that will
be necessary to use *Type2* and *Signature* throughout instead
of the old *StaticType* representations.
Signed-off-by: Mike Samuel <mikesamuel@gmail.com>
Signed-off-by: Mike Samuel <mikesamuel@gmail.com>
Signed-off-by: Mike Samuel <mikesamuel@gmail.com>
| translateDotHelperCall( | ||
| tree, | ||
| callee = effectiveCallee, | ||
| outerCallTree = null, |
There was a problem hiding this comment.
There is no outer call tree.
| actualCalleeType = hackTryStaticTypeToSig(callTree.childOrNull(0)?.typeInferences?.type) | ||
| } else if (dotHelper.memberAccessor is BindMemberAccessor) { | ||
| val innerCallee = callTree.childOrNull(0) | ||
| val declaredCalleeType: Signature2? = firstMember?.descriptor |
There was a problem hiding this comment.
Still want to store the chosen MemberShape with DotHelper post typing. That's a PR for a different time though.
There was a problem hiding this comment.
Some indenting changed, so recommend viewing in -w mode.
| val actualCalleeType: Signature2? = hackTryStaticTypeToSig(callTree.childOrNull(0)?.typeInferences?.type) | ||
|
|
||
| actualCalleeType = innerCallee?.typeInferences?.type?.let { | ||
| hackTryStaticTypeToSig(BindMethodTypeHelper.uncurry(it)) |
There was a problem hiding this comment.
No need to uncurry anything anymore.
| // Replace the macro call with regex constructor calls. | ||
| val callPos = macroEnv.pos | ||
| macroEnv.replaceMacroCallWith { | ||
| Call { |
There was a problem hiding this comment.
Lots of this kind of change to code that produces method calls: the outer call wrapper goes away in favour of just the inner call which adopts the extra arguments.
Before (Call (Call do_bind_foo subject) arg)
After (Call do_bind_foo subject arg)
| enum class TypeOpPrecedence { | ||
| Or, // Binds loosest | ||
| And, | ||
| Fn, |
There was a problem hiding this comment.
Ran into a minor bug in debug output where, in a nullable function type, the ? was glomming onto the end of a function-type string representation.
Added tests to StaticTypeTest.
| else -> false | ||
| } | ||
|
|
||
| val StaticType.nullity: Nullity get() = when (this) { |
There was a problem hiding this comment.
Mirrors the nullity member on Type2.
There was a problem hiding this comment.
Reworked the code that turns DotHelper calls back into subject.method(...) syntax when resugar options are on.
I consolidated some explicit and implicit type arg stuff too.
Added tests to PseudoCodeTest.
| ) | ||
| val strToVoidOrNull = MkType.nullable(strToVoid) | ||
| assertStringsEqual( | ||
| "(fn (String): Void)?", |
There was a problem hiding this comment.
This was coming out as fn (String): Void? which is confusing.
| } | ||
| } | ||
|
|
||
| private fun assertResult( |
There was a problem hiding this comment.
Moved this down. I prefer to have all the test helpers either at the top or the bottom.
|
Some breakage in C# and Rust backends which I think are related to tweaks to the declared type derivation in translateDotHelper. I think the C# side exposes a legit issue, see #448. The Rust side I think is a problem with this PR. Going to differentially check declared/actual type with printf debugging in some of the Rust functional tests before and after this change. Maybe will try to boil it down to a RustBackendTest case first. |
|
The Rust failure is in this code from json.temper.md: let decodeHexUnsigned(
sourceText: String, start: StringIndex, limit: StringIndex
): Int {
var n = 0;
var i = start;
while (i.compareTo(limit) < 0) {
let cp = sourceText[i];
let digit = if (char'0' <= cp && cp <= char'0') {
cp - char'0'
} else if (char'A' <= cp && cp <= char'F') {
cp - char'A' + 10
} else if (char'a' <= cp && cp <= char'f') {
cp - char'a' + 10
} else {
return -1;
}
n = (n * 16) + digit;
i = sourceText.next(i);
}
n
}The problem happens in My guess is that the problem comes from |
|
Before these changes: After: Strangely, it's the |
Signed-off-by: Mike Samuel <mikesamuel@gmail.com>
Signed-off-by: Mike Samuel <mikesamuel@gmail.com>
|
Merged in main which fixes C# failure. |
Signed-off-by: Mike Samuel <mikesamuel@gmail.com>
| |{ | ||
| | builder__0 = new C::OrderedDictionary<string, string>(); | ||
| | Map1 = C::Mapped.ToMap(builder__0); | ||
| | Map1 = C::Mapped.ToMap(C::Mapped.AsReadOnly(builder__0)); |
There was a problem hiding this comment.
Not sure whether this extra cast here a problem?
Signed-off-by: Tom <tom@temper.systems>
Signed-off-by: Tom <tom@temper.systems>
- Rely on fixed StringIndexOption compare typing - Expect more inlining in a backend test
tjpalmer
left a comment
There was a problem hiding this comment.
Looks like lots of the logic got simpler, which is nice. Overall, I prefer the new way.
| for (var i = 0; i < 3; ++i) | ||
| console.log(i.toString()) // Curly brackets missing | ||
| // ❌ Expected function type, but got Invalid!, No type for subject of .toString!, Expected function type, but got Invalid! | ||
| // ❌ Expected function type, but got Function!, No type for subject of .toString! |
There was a problem hiding this comment.
Expected function type, but got Function!, No type for subject of .toString!
That "but got Function" is a bit unfortunate, but we can refine later. That second message is helpful. And only two messages instead of three is also nice.
| membersGrouped.removeMatching { it.value.isEmpty() } | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
This is redoing some of the analysis you did in auto-inserting super calls.
Maybe we can clean out some of the later logic at some point.
| * | ||
| * Given these two alternative signatures, `fn (K): String` and `fn (I<String>): String` | ||
| * the type solver has no way to pick between the two leading to an unresolvable overload | ||
| * decision. |
There was a problem hiding this comment.
Or if we choose to error more in the frontend, that would also let us clean back out some from the backends. Since we don't promise semantics when there are frontend errors.
But I guess we probably don't want to error unless we have a manual way to override super selection. Or we could just say "too bad, use composition instead of inheritance here".
| * Then we can compare the signatures for equality (ignoring the `this` arg types), | ||
| * and since they're equivalent, drop any but the shallowest tier ones. | ||
| */ | ||
| internal fun filterOutDeeperMembers( |
There was a problem hiding this comment.
I can't tell if inheritance filtering need is increased by this PR or if you just happened to notice it as interesting. And maybe you already explained that elsewhere. I'm ok either way. Just hard to grok everything in review.
| | interface C { a(): String { "C wins!" } } | ||
| | class D extends B & C {} | ||
| | new D().a() | ||
| |/// ┗━━━━━━━━━┛ : String |
There was a problem hiding this comment.
How can we tell which wins from this test? Or did that involve manual inspection? Either way, I'm not worried. Just wondering if I'm missing something.
There was a problem hiding this comment.
It doesn't. I just copied your style of test.
I'd like to get to the point where we're storing member shapes with dot helpers post typing.
Then, maybe I could add a way to point at a dot and use the comment style to point out which member is stored as a way of checking @overload @operator @extension and super-type resolution.
| // We resolved types just fine, but parameterized methods in interfaces are still suss. | ||
| wantErrors = listOf( | ||
| "1+20: Illegal type parameter T. Overridable methods don't allow generics!", | ||
| "3+20: Illegal type parameter T. Overridable methods don't allow generics!", |
There was a problem hiding this comment.
I wouldn't worry too much about nice handling of type formals that are already illegal.
Previously, method calls were represented using separate bind and call steps.
There's something elegant to treating complex calls like the below as
multiple distinct steps.
It was convenient to type each part separately.
MyClass<Bar><U>(MyClass<T>) -> (U, U) -> Foo<T, U>,so when bound to
MyClass<Bar>the T binds to Bar and the overall typefor the bound method has type
<U>(U, U) -> Foo<Bar, U>method has type
(TypeActual, TypeActual) -> Foo<Bar, TypeActual>Foo<Bar, TypeActual>.That only works if you can represent calls to overloaded methods as
one type, an intersection (
&) type of function types. Note alsothat the type of the bound method is a function type that returns a
function type. There's currying going on. Code reasoning about this
has to repeatedly pack and unpack intersection types and curry and
uncurry type info.
Translating intersection types is hard; most typed languages
(TypeScript being an exception) do not allow user's to explicitly
specify intersection types, though many use them under the hood.
Also, reasoning about whole calls is more common than reasoning about the
parts above (again, outside of the Typer), so many parts of the frontend and
the TmpL translator were complicated by the need to look through calls
so that the could reassemble full argument lists (subject and args)
across two calls.
Ongoing changes to the internal representation of types and to type
solving made it easier to sequester overload handling to a small
portion of the Typer and TypeSolver, obviating the need for
intersection types, and the need for union types (previously used for
nullable type representation and unions of success and failure types
in result types).
This commit reworks DotHelper; instead of BindMemberAccessor
specifying bind semantics, now CallMemberAccessor specifies full
call semantics.
There is a lot of cleanup and reworking of Typer internally to
type the new representation, and cleanup of existing code that
produces method calls to produce the newer, flatter calls.
A lot of this reworking of the typer prefigures work that will
be necessary to use Type2 and Signature throughout instead
of the old StaticType representations.
Signed-off-by: Mike Samuel mikesamuel@gmail.com