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

Consolidate Interlocked initialize implementations #69017

Merged
merged 3 commits into from
Jul 19, 2023

Conversation

sharwell
Copy link
Member

  • Merge LazyInitialization into InterlockedOperations (RoslynLazyInitializer was not appropriate as that type is a mirror of LazyInitializer for nullable reference types support)
  • Update implementations for consistency (initialization methods all named Initialize) and clarity (helper method named GetOrStore)

@sharwell sharwell requested review from a team as code owners July 13, 2023 12:31
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Jul 13, 2023
@AlekseyTs

This comment was marked as resolved.

@sharwell sharwell changed the title Interlocked initialize Consolidate Interlocked initialize implementations Jul 13, 2023
Initialization calls where the initialized value is null and the
uninitialized value is non-null became ambiguous once additional
initialization patterns were implemented. Since this is only used in one
situation in each of C# and VB compilers, we resolve it by providing an
explicit argument name for the initialized value.
@jaredpar jaredpar added this to the 17.8 milestone Jul 13, 2023
@sharwell
Copy link
Member Author

@dotnet/roslyn-compiler for second review

1 similar comment
@sharwell
Copy link
Member Author

@dotnet/roslyn-compiler for second review

@@ -1549,7 +1549,7 @@ internal override ObsoleteAttributeData ObsoleteAttributeData
{
var result = uncommonFields._lazyObsoleteAttributeData;
return ReferenceEquals(result, ObsoleteAttributeData.Uninitialized)
? InterlockedOperations.Initialize(ref uncommonFields._lazyObsoleteAttributeData, null, ObsoleteAttributeData.Uninitialized)
? InterlockedOperations.Initialize(ref uncommonFields._lazyObsoleteAttributeData, initializedValue: null, ObsoleteAttributeData.Uninitialized)
Copy link
Contributor

@RikkiGibson RikkiGibson Jul 19, 2023

Choose a reason for hiding this comment

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

This may overlap/conflict with changes in #68930. Heads up @cston.

Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't find any overlap/conflict

Copy link
Contributor

Choose a reason for hiding this comment

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

I think there actually isn't any. I think the linked PR is using Interlocked.CompareExchange, not any InterlockedOperations or similar APIs that changed in this PR.

@sharwell sharwell merged commit 86d0fd2 into dotnet:main Jul 19, 2023
27 checks passed
@sharwell sharwell deleted the interlocked-init branch July 19, 2023 22:25
@ghost ghost modified the milestones: 17.8, Next Jul 19, 2023
@allisonchou allisonchou modified the milestones: Next, 17.8 P1 Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants