Reset partial memberlist on defered circularity to calculate the correct members#20179
Merged
Conversation
rbuckton
reviewed
Nov 21, 2017
|
|
||
| function resolveBaseTypesOfClass(type: InterfaceType): void { | ||
| type.resolvedBaseTypes = emptyArray; | ||
| function resolveBaseTypesOfClass(type: InterfaceType): BaseType[] { |
Contributor
There was a problem hiding this comment.
It doesn't look like you are using this return type.
Member
Author
There was a problem hiding this comment.
No, I was just using it to ensure I set resolvedBaseType at each exit point. I can replace it with void if you'd like.
errendir
added a commit
to errendir/TypeScript
that referenced
this pull request
Jan 7, 2018
* origin/master: (216 commits) Accepted baselines. Check whether we have a class before testing whether we have a super-class. Added tests. LEGO: check in for master to temporary branch. LEGO: check in for master to temporary branch. Add method signature handler to getTypeOfVariableOrParameterOrProperty (microsoft#20825) LEGO: check in for master to temporary branch. Simplify test and add explanatory assertion LEGO: check in for master to temporary branch. Add dynamic file open test Allow dynamic files script info to be created when not opened by client LEGO: check in for master to temporary branch. Accept new baselines Add regression test Fix narrowTypeBySwitchOnDiscriminant function LEGO: check in for master to temporary branch. LEGO: check in for master to temporary branch. Move recursion limiter to individual resolve* functions User runner submodule improvements (microsoft#20868) Reset partial memberlist on defered circularity to calculate the correct members (microsoft#20179) ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #16861
We've had this bug since we released generic defaults, according to my
bisecttest. The root cause of the assertion violation is anundefinedsymbol which we expect to be present because of its presence in the class inheritance heirarchy. It is missing because the.memberscache of members was calculated while thebaseTypeof the class was still resolving (as the generic defaultC["someProp"]forces a resolution of them), and that partial list is kept around and reused even once the fully resolved inheritance hierarchy is known. In this PR, I simply clear that cache when I know it is missing values, as the recurrence is valid if the index does not access a member whose type is itself recursive (which will trigger a different recursion guard).Another option (rather than resetting the cache we know is bad) is to issue a circularity error - however since the circularity is indirect (via the index access), I believe it's fine to allow it so long as we acknowledge the partially created member cache we create prior to knowing that the type affects its own base - then deal with it appropriately. The most correct thing to do would probably be making
instantiateTypefor an indexed access not instantiate the whole object type (and thereby any base types it may have) before callinggetPropertyOfTypeon it, and instead attempt to just instantiate the single target symbol - there may even be a perf benefit to doing so.