Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make frozen doc state non-nullable #77896

Merged
merged 6 commits into from
Mar 31, 2025

Conversation

CyrusNajmabadi
Copy link
Member

No description provided.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner March 28, 2025 18:25
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Mar 28, 2025
@CyrusNajmabadi
Copy link
Member Author

Trying something out to see if it cleans up compilation tracker state management.

@CyrusNajmabadi
Copy link
Member Author

@jasonmalinowski @davidwengier ptal.

@CyrusNajmabadi CyrusNajmabadi changed the title WIP: Make frozen doc state non-nullable Make frozen doc state non-nullable Mar 28, 2025
@CyrusNajmabadi CyrusNajmabadi requested a review from dibarbet March 28, 2025 22:28
@CyrusNajmabadi
Copy link
Member Author

@dibarbet ptal.

Comment on lines 141 to 142
Contract.ThrowIfFalse(FrozenSourceGeneratedDocumentIdentities.HasValue);
Contract.ThrowIfFalse(FrozenSourceGeneratedDocuments.HasValue);
Copy link
Member

Choose a reason for hiding this comment

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

If these should now always have a value, should they become non-nullable too?

Copy link
Member Author

Choose a reason for hiding this comment

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

So much simpler. Thanks!

@CyrusNajmabadi
Copy link
Member Author

Copy link
Member

@davidwengier davidwengier left a comment

Choose a reason for hiding this comment

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

The changes from something being nullable to non-nullable make sense to me. Whether the scenario still makes sense I leave for someone else to work out :)

@CyrusNajmabadi CyrusNajmabadi merged commit c1c78fb into dotnet:release/dev17.15 Mar 31, 2025
25 checks passed
@CyrusNajmabadi CyrusNajmabadi deleted the frozenDocs branch March 31, 2025 22:41
@@ -117,7 +117,7 @@ private SolutionCompilationState Branch(
SolutionState newSolutionState,
ImmutableSegmentedDictionary<ProjectId, ICompilationTracker>? projectIdToTrackerMap = null,
SourceGeneratorExecutionVersionMap? sourceGeneratorExecutionVersionMap = null,
Optional<TextDocumentStates<SourceGeneratedDocumentState>?> frozenSourceGeneratedDocumentStates = default,
Optional<TextDocumentStates<SourceGeneratedDocumentState>> frozenSourceGeneratedDocumentStates = default,
Copy link
Member

Choose a reason for hiding this comment

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

You don't need Optional here anymore.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call. Simplifying.

// For the frozen source generated document info, we expect two either have both checksum collections or neither, and they
// should both be the same length as there is a 1:1 correspondence between them.
Contract.ThrowIfFalse(frozenSourceGeneratedDocumentIdentities.HasValue == frozenSourceGeneratedDocuments.HasValue);
Contract.ThrowIfFalse(frozenSourceGeneratedDocumentIdentities?.Count == frozenSourceGeneratedDocuments?.Length);
Copy link
Member

Choose a reason for hiding this comment

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

Did you still want to keep this assertion that the lengths match?

Copy link
Member Author

Choose a reason for hiding this comment

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

will add back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead VSCode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants