Skip to content

Allow late bound symbols in augmentations to merge into late bound symbol tables#29015

Closed
weswigham wants to merge 3 commits into
microsoft:masterfrom
weswigham:unique-symbol-assignability-checking
Closed

Allow late bound symbols in augmentations to merge into late bound symbol tables#29015
weswigham wants to merge 3 commits into
microsoft:masterfrom
weswigham:unique-symbol-assignability-checking

Conversation

@weswigham
Copy link
Copy Markdown
Member

Fixes #28334

@weswigham
Copy link
Copy Markdown
Member Author

ping @rbuckton for a review~

Comment thread src/compiler/checker.ts
return links.resolvedSymbol = lateSymbol;
}
}
else {
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.

Part of the reason to have a resolvedSymbol in the first place is to avoid doing the same work over and over again. I'm concerned that this code path would have a performance impact.

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.

This is within lateBindMember - we're just saying that if we attempt to late bind a symbol and the symbol is already resolved, but for some reason not in the late bound symbol table (eg, because it came from a global augmentation), that we add it to the late bound symbol table to it can actually be looked up by name later.

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.

You should check that resolvedSymbol is CheckFlags.Late, since we first assign it to decl.symbol to avoid recursion: https://github.com/Microsoft/TypeScript/pull/29015/files#diff-c3ed224e4daa84352f7f1abcd23e8ccaR6577. Otherwise, you might accidentally assign the wrong symbol in the lateSymbols table.

@rbuckton
Copy link
Copy Markdown
Contributor

rbuckton commented Mar 8, 2019

Your build is failing, can you take a look.

@weswigham
Copy link
Copy Markdown
Member Author

Yeah, looks like one of the newer baselines I've added is affected by this; I'll have to go gin up the diff and add it.

Comment thread src/compiler/checker.ts
return links.resolvedSymbol = lateSymbol;
}
}
else {
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.

You should check that resolvedSymbol is CheckFlags.Late, since we first assign it to decl.symbol to avoid recursion: https://github.com/Microsoft/TypeScript/pull/29015/files#diff-c3ed224e4daa84352f7f1abcd23e8ccaR6577. Otherwise, you might accidentally assign the wrong symbol in the lateSymbols table.

@weswigham
Copy link
Copy Markdown
Member Author

Handled by #30970

@weswigham weswigham closed this Apr 17, 2019
@weswigham weswigham deleted the unique-symbol-assignability-checking branch April 17, 2019 23:49
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
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.

No index signature error when a global generic type has a symbol property

2 participants