Skip to content

Matching Strada order of operations in recursive signature resolution #64014

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.

Sign up for GitHub

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

Open
Ryan Cavanaugh (RyanCavanaugh) wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Matching Strada order of operations in recursive signature resolution #64014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions tsc/internal/checker/checker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8520,14 +8520,6 @@ func (c *Checker) getResolvedSignature(node *ast.Node, candidatesOutArray *[]*Si
// When CheckMode.SkipGenericFunctions is set we use resolvingSignature to indicate that call
// resolution should be deferred.
if result != c.resolvingSignature {
// if the signature resolution originated on a node that itself depends on the contextual type
// then it's possible that the resolved signature might not be the same as the one that would be computed in source order
// since resolving such signature leads to resolving the potential outer signature, its arguments and thus the very same signature
// it's possible that this inner resolution sets the resolvedSignature first.
// In such a case we ignore the local result and reuse the correct one that was cached.
if links.resolvedSignature != c.resolvingSignature {
result = links.resolvedSignature
}
// If signature resolution originated in control flow type analysis (for example to compute the
// assigned type in a flow assignment) we don't cache the result as it may be based on temporary
// types from the control flow analysis.
Expand Down Expand Up @@ -9016,6 +9008,13 @@ func (c *Checker) resolveCall(node *ast.Node, signatures []*Signature, candidate
if result == nil {
result = c.chooseOverload(&s, c.assignableRelation)
}
links := c.signatureLinks.Get(node)
if links.resolvedSignature != c.resolvingSignature && candidatesOutArray == nil {
// Signature resolution may reenter for the same node and cache the source-order result.
// Prefer that result over one computed using an incomplete contextual type.
debug.Assert(links.resolvedSignature != nil)
return links.resolvedSignature

@Andarist Mateusz Burzyński (Andarist) Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving it like this here breaks:

declare const example: (f: (a: string) => number) => string;

const f = (a: string) => g();

const g = () => {
  return example(f);
};

The above should error but IIRC it doesn't with this fix. Note that I'm mainly speaking from my memory right now - but this change basically ports my own #60208 and I was looking into this whole issue yesterday (and still thinking about how to fix it to satisfy all the cases) so I hope my memory serves me well ;p

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Aug 26, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That isn't an error in 6.0 either, which makes sense since this is just trying to copy the Strada behavior

@Andarist Mateusz Burzyński (Andarist) Aug 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ye, right - but it errors correctly in Corsa right now. I can open a separate issue about it later - I just wanted to mention the fix here has its drawbacks

}
if result != nil {
return result
}
Expand All @@ -9027,7 +9026,7 @@ func (c *Checker) resolveCall(node *ast.Node, signatures []*Signature, candidate
// don't hit this issue because they only observe this result after it's had a chance to
// be cached, but the error reporting code below executes before getResolvedSignature sets
// resolvedSignature.
c.signatureLinks.Get(node).resolvedSignature = result
links.resolvedSignature = result
// No signatures were applicable. Now report errors based on the last applicable signature with
// no arguments excluded from assignability checks.
// If candidate is undefined, it means that no candidates had a suitable arity. In that case,
Expand Down
36 changes: 36 additions & 0 deletions tsc/testdata/baselines/reference/compiler/genericDefaultInContextSensitiveCallback.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
//// [tests/cases/compiler/genericDefaultInContextSensitiveCallback.ts] ////

=== genericDefaultInContextSensitiveCallback.ts ===
declare function request<T = { input: { value: number }; output: string }>(
>request : Symbol(request, Decl(genericDefaultInContextSensitiveCallback.ts, 0, 0))
>T : Symbol(T, Decl(genericDefaultInContextSensitiveCallback.ts, 0, 25))
>input : Symbol(input, Decl(genericDefaultInContextSensitiveCallback.ts, 0, 30))
>value : Symbol(value, Decl(genericDefaultInContextSensitiveCallback.ts, 0, 39))
>output : Symbol(output, Decl(genericDefaultInContextSensitiveCallback.ts, 0, 56))

body?: T extends { input: unknown } ? T["input"] : never,
>body : Symbol(body, Decl(genericDefaultInContextSensitiveCallback.ts, 0, 75))
>T : Symbol(T, Decl(genericDefaultInContextSensitiveCallback.ts, 0, 25))
>input : Symbol(input, Decl(genericDefaultInContextSensitiveCallback.ts, 1, 22))
>T : Symbol(T, Decl(genericDefaultInContextSensitiveCallback.ts, 0, 25))

): T extends { output: unknown } ? T["output"] : T;
>T : Symbol(T, Decl(genericDefaultInContextSensitiveCallback.ts, 0, 25))
>output : Symbol(output, Decl(genericDefaultInContextSensitiveCallback.ts, 2, 14))
>T : Symbol(T, Decl(genericDefaultInContextSensitiveCallback.ts, 0, 25))
>T : Symbol(T, Decl(genericDefaultInContextSensitiveCallback.ts, 0, 25))

declare function identity<T>(callback: T): T;
>identity : Symbol(identity, Decl(genericDefaultInContextSensitiveCallback.ts, 2, 51))
>T : Symbol(T, Decl(genericDefaultInContextSensitiveCallback.ts, 4, 26))
>callback : Symbol(callback, Decl(genericDefaultInContextSensitiveCallback.ts, 4, 29))
>T : Symbol(T, Decl(genericDefaultInContextSensitiveCallback.ts, 4, 26))
>T : Symbol(T, Decl(genericDefaultInContextSensitiveCallback.ts, 4, 26))

identity((data: { value: number }) => request(data));
>identity : Symbol(identity, Decl(genericDefaultInContextSensitiveCallback.ts, 2, 51))
>data : Symbol(data, Decl(genericDefaultInContextSensitiveCallback.ts, 6, 10))
>value : Symbol(value, Decl(genericDefaultInContextSensitiveCallback.ts, 6, 17))
>request : Symbol(request, Decl(genericDefaultInContextSensitiveCallback.ts, 0, 0))
>data : Symbol(data, Decl(genericDefaultInContextSensitiveCallback.ts, 6, 10))

30 changes: 30 additions & 0 deletions tsc/testdata/baselines/reference/compiler/genericDefaultInContextSensitiveCallback.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
//// [tests/cases/compiler/genericDefaultInContextSensitiveCallback.ts] ////

=== genericDefaultInContextSensitiveCallback.ts ===
declare function request<T = { input: { value: number }; output: string }>(
>request : <T = { input: { value: number; }; output: string; }>(body?: T extends { input: unknown; } ? T["input"] : never) => T extends { output: unknown; } ? T["output"] : T
>input : { value: number; }
>value : number
>output : string

body?: T extends { input: unknown } ? T["input"] : never,
>body : (T extends { input: unknown; } ? T["input"] : never) | undefined
>input : unknown

): T extends { output: unknown } ? T["output"] : T;
>output : unknown

declare function identity<T>(callback: T): T;
>identity : <T>(callback: T) => T
>callback : T

identity((data: { value: number }) => request(data));
>identity((data: { value: number }) => request(data)) : (data: { value: number; }) => string
>identity : <T>(callback: T) => T
>(data: { value: number }) => request(data) : (data: { value: number; }) => string
>data : { value: number; }
>value : number
>request(data) : string
>request : <T = { input: { value: number; }; output: string; }>(body?: T extends { input: unknown; } ? T["input"] : never) => T extends { output: unknown; } ? T["output"] : T
>data : { value: number; }

10 changes: 10 additions & 0 deletions tsc/testdata/tests/cases/compiler/genericDefaultInContextSensitiveCallback.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// @strict: true
// @noEmit: true

declare function request<T = { input: { value: number }; output: string }>(
body?: T extends { input: unknown } ? T["input"] : never,
): T extends { output: unknown } ? T["output"] : T;

declare function identity<T>(callback: T): T;

identity((data: { value: number }) => request(data));