Skip to content
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

Deeper type inference in context sensitive arguments #1861

Merged
merged 5 commits into from
Feb 2, 2015

Conversation

ahejlsberg
Copy link
Member

Fixes #1181.

Previously, context sensitive arguments (arguments containing function expressions that are subject to contextual typing) would be completely ignored in the first round of type inference. With this change we instead infer from all arguments in the first round, but treat context sensitive function expressions as wildcards from which no inferences are made.

interface Computed<T> {
    read(): T;
    write(value: T);
}

function foo<T>(x: Computed<T>) { }

var s: string;

foo({
    read: () => s,
    write: value => s = value
});

This example would previously infer {} for T because the object literal argument was excluded from the first round of type inference. We now include the argument in the first round, but treat the context sensitive arrow function value => s = value as a wildcard. That allows us to infer string for T from the (non-sensitive) arrow function () => s. In the second round of inference we then end up fixing T and assigning the inferred type string to the value parameter.

This also fixes a bug related to #1648 where the contextualMapper argument wasn't being propagated into a parenthesized expression, causing us to leak a type parameter.

// First infer from arguments that are not context sensitive
var inferenceMapper = createInferenceMapper(context);

// First infer from all arguments using wildcards for all context sensitive function expressions
Copy link
Member

Choose a reason for hiding this comment

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

Make this comment First infer from all arguments, but use wildcards for all context sensitive expressions.

The "but" actually would really help to point out that this is a special case.

@DanielRosenwasser
Copy link
Member

It is strange that we only have 2 affected baselines, and only in one of them did we see that leaked type parameter issue. This change really needs more test coverage.

@JsonFreeman
Copy link
Contributor

Please add tests for both of these changes.

@JsonFreeman
Copy link
Contributor

I'm fine with this code change, but it needs tests.
Also, I do think that the related issues we discussed should be addressed soon.

@JsonFreeman
Copy link
Contributor

👍

ahejlsberg added a commit that referenced this pull request Feb 2, 2015
Deeper type inference in context sensitive arguments
@ahejlsberg ahejlsberg merged commit 3f3e974 into master Feb 2, 2015
@ahejlsberg ahejlsberg deleted the deeperTypeInference branch February 2, 2015 19:05
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression in type resolution post-1.0
4 participants