Skip to content

Fix rename test flakeyness with dynamic types.#43631

Merged
2 commits merged intodotnet:masterfrom
CyrusNajmabadi:dynamicTypes
Apr 24, 2020
Merged

Fix rename test flakeyness with dynamic types.#43631
2 commits merged intodotnet:masterfrom
CyrusNajmabadi:dynamicTypes

Conversation

@CyrusNajmabadi
Copy link
Contributor

No description provided.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner April 24, 2020 04:38
// assembly). So we have to keep track of it so we can get back from it to a project in case the
// underlying compilation is GC'ed.
if (compilation.Language == LanguageNames.CSharp)
result.Add(compilation.DynamicType, null);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

language check is deliberate as VB does not support this and just throws.

foreach (var (projectId, tracker) in _projectIdToTrackerMap)
{
// VB doesn't have DynamicTypes (and throws if you ask for them), so just check C# projects.
if (tracker.TryGetCompilation(out var compilation) &&
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was hte failure point from before. it was possible to hold onto a dynamic type, while it's compilation could still get GC'ed underneath it. so later when we walked the solution looking for it, we wouldn't get a compilatino back from any tracker, and so we wouldn't find this symbol.

Copy link
Contributor

@sharwell sharwell left a comment

Choose a reason for hiding this comment

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

Would be good to have a second review. I feel like I mostly understand this but it's a new area for me.

…SymbolToProjectId.cs

Co-Authored-By: Sam Harwell <sam@tunnelvisionlabs.com>
@CyrusNajmabadi
Copy link
Contributor Author

Tagging @jasonmalinowski as well as he reviewed the initial PR introducing the back-references from symbol to projects.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Auto-approval

@CyrusNajmabadi
Copy link
Contributor Author

Would like to move forward on this to aid with CI. But happy to make more changes if people want them.

@ghost ghost merged commit 2786fba into dotnet:master Apr 24, 2020
@CyrusNajmabadi CyrusNajmabadi deleted the dynamicTypes branch April 29, 2020 22:59
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants