Skip to content

Generate a unique type parameter name for each nested type parameter#31544

Merged
weswigham merged 5 commits into
microsoft:masterfrom
weswigham:revise-type-param-namegen
Jul 11, 2019
Merged

Generate a unique type parameter name for each nested type parameter#31544
weswigham merged 5 commits into
microsoft:masterfrom
weswigham:revise-type-param-namegen

Conversation

@weswigham
Copy link
Copy Markdown
Member

@weswigham weswigham commented May 22, 2019

Previously we'd generate a name for each declaration we saw, but this neglected to account of the possibility that we see multiple unique instantiations of the same symbol. Now the name is generated in the node builder, rather than deferring to the emitter, so we can have a unique name for each type.

Fixes half of the issues brought to light by #30732.

Fixes #31605.

@ajafff
Copy link
Copy Markdown
Contributor

ajafff commented Jun 3, 2019

Will this also fix #30373? It looks similar to the other issue.

@weswigham
Copy link
Copy Markdown
Member Author

Quite possibly. Quickinfo does use the same machinery for printing types.

@weswigham weswigham force-pushed the revise-type-param-namegen branch from 0831876 to 6aa8ea4 Compare June 26, 2019 22:25
@weswigham
Copy link
Copy Markdown
Member Author

@typescript-bot test this
@typescript-bot user test this
@typescript-bot run dt

@typescript-bot
Copy link
Copy Markdown
Collaborator

typescript-bot commented Jun 26, 2019

Heya @weswigham, I've started to run the extended test suite on this PR at 6aa8ea4. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Copy Markdown
Collaborator

typescript-bot commented Jun 26, 2019

Heya @weswigham, I've started to run the community code test suite on this PR at 6aa8ea4. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Copy Markdown
Collaborator

typescript-bot commented Jun 26, 2019

Heya @weswigham, I've started to run the parallelized Definitely Typed test suite on this PR at 6aa8ea4. You can monitor the build here. It should now contribute to this PR's status checks.

@weswigham weswigham requested a review from RyanCavanaugh June 26, 2019 22:30
@weswigham
Copy link
Copy Markdown
Member Author

RWC changes are minor - we now use some _1's in the printout whereas before we didn't (the node builder doesn't have a concept of sibling scopes, so generated names are reserved globally within a node builder invocation).

@weswigham
Copy link
Copy Markdown
Member Author

DT failure is unrelated (a module seems to have gone missing from DT), so looks good.

@typescript-bot
Copy link
Copy Markdown
Collaborator

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

@weswigham
Copy link
Copy Markdown
Member Author

User suite failure is just a new package in the azure-sdk-for-js that hasn't made it into master yet, so is clean. Tests are 💯

@weswigham weswigham requested a review from sandersn July 10, 2019 23:19
@weswigham weswigham merged commit 6839973 into microsoft:master Jul 11, 2019
@weswigham weswigham deleted the revise-type-param-namegen branch July 11, 2019 00:12
@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.

Incorrect Recursive Union Emmission

4 participants