Skip to content

Move convert tuple to struct oop#43771

Merged
CyrusNajmabadi merged 4 commits intodotnet:masterfrom
CyrusNajmabadi:convertTupleToStructOOP
Apr 29, 2020
Merged

Move convert tuple to struct oop#43771
CyrusNajmabadi merged 4 commits intodotnet:masterfrom
CyrusNajmabadi:convertTupleToStructOOP

Conversation

@CyrusNajmabadi
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi commented Apr 29, 2020

This feature uses the SyntaxIndex type. So we want this to run OOP so that those indices are never produces in-proc.

Reviewing commit by commit may help.

document, span, scope, cancellationToken).ConfigureAwait(false);
}

private async Task<Solution> AddRenameTokenAsync(
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, so annotations get lost during serialization? Is it why we need to keep track of them here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. We'd like to address that in a later change, but it will be a significant amount of work to support.

@CyrusNajmabadi CyrusNajmabadi requested a review from genlu April 29, 2020 21:41
@CyrusNajmabadi CyrusNajmabadi requested a review from genlu April 29, 2020 21:49
Copy link
Member

@genlu genlu 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 3a3029d into dotnet:master Apr 29, 2020
@ghost ghost added this to the Next milestone Apr 29, 2020
@CyrusNajmabadi CyrusNajmabadi deleted the convertTupleToStructOOP branch April 29, 2020 21:57
{
cancellationToken.ThrowIfCancellationRequested();

using (Logger.LogBlock(FunctionId.AbstractConvertTupleToStructCodeRefactoringProvider_ConvertToStructAsync, cancellationToken))
Copy link
Member

Choose a reason for hiding this comment

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

gdpr approval reminder :)

// in the concrete type we create.
var fields = tupleType.TupleElements;
var containsAnonymousType = fields.Any(p => p.Type.ContainsAnonymousType());
var containsAnonymousType = fields.Any<IFieldSymbol>(p => p.Type.ContainsAnonymousType());
Copy link
Member

Choose a reason for hiding this comment

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

I see here and in the rest of the file we're adding the type parameters to all the linq methods, what is the reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see above. it's a bug we have in our complexificatin/simplificaiton system. have cleaned up in followup PR.

}
}

return await ConvertToStructInCurrentProcessAsync(
Copy link
Member

Choose a reason for hiding this comment

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

do we need to keep around the in-proc one? And if we do, should we log any issues if we can't use the OOP version?

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 always need hte "in proc" one since when we call to OOP, OOP then runs the actual code "in proc" (of itself) :)

var document = solution.GetDocument(documentId);

var service = document.GetLanguageService<IConvertTupleToStructCodeRefactoringProvider>();
var updatedSolution = await service.ConvertToStructAsync(document, span, scope, cancellationToken).ConfigureAwait(false);
Copy link
Member

Choose a reason for hiding this comment

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

should this call just return the serializable results by default? the inproc version and the oop version could convert back to sln as needed

@jinujoseph jinujoseph added Area-IDE Concept-OOP Related to out-of-proc labels Apr 30, 2020
@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

Area-IDE Concept-OOP Related to out-of-proc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants