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

Union subtype reduction #4537

Merged
merged 4 commits into from
Aug 29, 2015
Merged

Union subtype reduction #4537

merged 4 commits into from
Aug 29, 2015

Conversation

ahejlsberg
Copy link
Member

This PR rolls back some of the changes related to union types introduced in #3823. Specifically, we now again use subtype reduction as the primary means of removing redundant constituent types in union types. This improves backwards compatibility without impacting the strict object literal assignment checking that was the primary purpose of #3823.

It turns out that subtype reduction isn't at odds with strict object literal assignment checks as long as we are a bit more selective about where we perform the reduction. Specifically, because fresh object literals are subtypes only of object types with an exact matching set of properties (the fresh object literals can have neither too many nor too few properties), we get pretty much the same behavior from subtype checks as we did from the new deduplication algorithm in #3823.

@ahejlsberg
Copy link
Member Author

@JsonFreeman Thought this might interest you!

// is considered known if the object type is empty and the check is for assignability, if the object type has
// index signatures, or if the property is actually declared in the object type. In a union or intersection
// type, a property is considered known if it is known in any constituent type.
function isKnownProperty(type: Type, name: string): boolean {
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps isExpectedProperty would better describe this.

@JsonFreeman
Copy link
Contributor

Whoa, interesting! That's true about fresh object literals failing the subtype test for subtype reduction. I have two general questions:

  1. Is there any concrete benefit to having subtype reduction (given the new way of computing signatures of union types), or is it just for simplicity - to avoid having an extra type relation?
  2. What is the difference between this incarnation of subtype reduction and the way it used to be? Presumably this fact about fresh object literals not getting thrown away would have applied before as well.

@mhegazy
Copy link
Contributor

mhegazy commented Aug 28, 2015

👍

function removeDuplicateTypes(types: Type[]) {
let i = types.length;
function removeSubtypes(types: Type[]) {
var i = types.length;
Copy link
Member

Choose a reason for hiding this comment

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

Use let

@ahejlsberg
Copy link
Member Author

Is there any concrete benefit to having subtype reduction (given the new way of computing signatures of union types), or is it just for simplicity - to avoid having an extra type relation?

It's mostly a backwards compatibility thing. We've seen several examples of real world code that was broken by the new behavior. The breaks all had to do with call signatures disappearing because a union type didn't get reduced to a single supertype. Of course it doesn't hurt that this new (or perhaps I should say old) way of doing things is also simpler in that there isn't a new kind of type relationship we have to document and reason about.

What is the difference between this incarnation of subtype reduction and the way it used to be? Presumably this fact about fresh object literals not getting thrown away would have applied before as well.

Consider this example:

var a = [{ x: 1 }, { x: 2, y: 2 }];  // ({ x: number } | { x: number, y: number })[]

Because the elements have fresh object literal types they aren't subtypes of each other, so the element type of the array literal is a union type. As we widen the type of the array literal we make sure _not_ to perform subtype reduction again as that would collapse the union when the object literal types lose their freshness. So, the full type survives as the type of a and the array literal is assignable to a. That previously wasn't the case (at one point in the evolution of #3823 we were in a state where the above was an error), and that's really the big difference.

mhegazy added a commit that referenced this pull request Aug 29, 2015
@mhegazy mhegazy merged commit 7f6608b into master Aug 29, 2015
@mhegazy mhegazy deleted the unionSubtypeReduction branch August 29, 2015 00:48
@JsonFreeman
Copy link
Contributor

@ahejlsberg Got it. Thanks for the explanation! So the difference is that noSubtypeReduction is true for the call to getUnionType in getWidenedType? That would seem to be the trap where subtype reduction is dangerous.

The scheme of aggregating the signatures of a union type is remaining as is, correct? I think it's still good for union types that didn't collapse under subtype reduction.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 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.

5 participants