Skip to content

Move dependent-type-finding operations out of process.#43636

Merged
CyrusNajmabadi merged 19 commits intodotnet:masterfrom
CyrusNajmabadi:depTypeFinderOOP
Apr 30, 2020
Merged

Move dependent-type-finding operations out of process.#43636
CyrusNajmabadi merged 19 commits intodotnet:masterfrom
CyrusNajmabadi:depTypeFinderOOP

Conversation

@CyrusNajmabadi
Copy link
Contributor

Followup to #43622. That PR should be reviewed first.

This moves the searching portion of the dependent-type-finder OOP allowing s to do that expensive work without impacting the main VS process.

@CyrusNajmabadi CyrusNajmabadi marked this pull request as ready for review April 25, 2020 06:29
@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner April 25, 2020 06:29
@CyrusNajmabadi CyrusNajmabadi requested a review from a team April 25, 2020 06:29
@CyrusNajmabadi
Copy link
Contributor Author

@jasonmalinowski This is ready for review now.

@@ -4,19 +4,58 @@

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renaming the file made this show as two different files, so i'm going to rename later.

@@ -174,7 +174,7 @@ private static async Task<ImmutableArray<INamedTypeSymbol>> DescendInheritanceTr
// We might miss a derived type in C if there's an intermediate derived type
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the changes in this file are followup to PR feedback from previous PR.

t => t.TypeKind == TypeKind.Class ||
t.TypeKind == TypeKind.Struct ||
t.TypeKind == TypeKind.Delegate ||
t.TypeKind == TypeKind.Enum);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

found by @jasonmalinowski when reviewing the the previous PR.

Renamer_ResolveConflictsAsync = 388,

ChangeSignature_Data = 389,
ChangeSignature_Data = 400,
Copy link
Member

Choose a reason for hiding this comment

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

400 [](start = 31, length = 3)

Shouldn't this be immutable once shipped? This is used for telemetry, so changing the values would render counters that span multiple version useless.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We haven't shipped any of these. These were all conflicts between having n prs trying to go through at the same time. I'm just trying to give these all some space as I keep needing to have to go back to features to add items and that means the numbers are all disconnected and the feature IDs are all scattered. This allows us to add ids really in a section in the future in a painless way

Copy link
Member

Choose a reason for hiding this comment

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

OK, as long as we haven't shipped any of these.


In reply to: 418199711 [](ancestors = 418199711)

// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Collections.Immutable;
Copy link
Member

@tmat tmat Apr 30, 2020

Choose a reason for hiding this comment

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

using [](start = 0, length = 5)

#nullable enable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will add in: #43804

// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
Copy link
Member

@tmat tmat Apr 30, 2020

Choose a reason for hiding this comment

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

System [](start = 6, length = 6)

#nullable enable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will add in #43804

@@ -0,0 +1,68 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.
Copy link
Member

@tmat tmat Apr 30, 2020

Choose a reason for hiding this comment

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

// [](start = 0, length = 2)

#nullable enable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will add in #43804

Copy link
Member

@tmat tmat left a comment

Choose a reason for hiding this comment

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

:shipit:

@CyrusNajmabadi CyrusNajmabadi merged commit da62ba8 into dotnet:master Apr 30, 2020
@ghost ghost added this to the Next milestone Apr 30, 2020
@CyrusNajmabadi CyrusNajmabadi deleted the depTypeFinderOOP branch April 30, 2020 19:25
@JoeRobich JoeRobich modified the milestones: Next, 16.7.P2 May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants