Skip to content

Fix crash in language service when using on inherited properties of self-extend#6227

Merged
yuit merged 9 commits intomasterfrom
fix4581_inheritedPropertiesCrash
Jan 14, 2016
Merged

Fix crash in language service when using on inherited properties of self-extend#6227
yuit merged 9 commits intomasterfrom
fix4581_inheritedPropertiesCrash

Conversation

@yuit
Copy link
Copy Markdown
Contributor

@yuit yuit commented Dec 23, 2015

Fix #4581

@DanielRosenwasser
Copy link
Copy Markdown
Member

This is not a sufficient fix. I can break this by creating an arbitrarily long loop of heritage clauses.

Here's an example:

interface C extends D {
    /*1*/propC: number;
}

interface D extends C {
    /*2*/propD: string;
}

Both occurrence locations cause a crash.

Comment thread src/services/services.ts Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think you'll just need to pass along a cache for this; also, can you make the result parameter come last?

Kanchalai Tanglertsampan added 2 commits January 8, 2016 03:34
@yuit
Copy link
Copy Markdown
Contributor Author

yuit commented Jan 13, 2016

@DanielRosenwasser any more comment?

Comment thread src/services/services.ts Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add a /*previousIterationSymbolsCache*/

@DanielRosenwasser
Copy link
Copy Markdown
Member

Apart from the comments: 🍪 🍰 🍬 💯

yuit added a commit that referenced this pull request Jan 14, 2016
Fix crash in language service when using on inherited properties of self-extend
@yuit yuit merged commit 9df1ed4 into master Jan 14, 2016
@yuit yuit deleted the fix4581_inheritedPropertiesCrash branch January 14, 2016 03:39
@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.

3 participants