-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Adjust IsMetadataVirtual assertion in MethodToClassRewriter #75066
Conversation
fbf84b4
to
c03b894
Compare
c03b894
to
ee1c7ab
Compare
@@ -658,12 +658,24 @@ internal virtual bool IsMetadataFinal | |||
/// signature). | |||
/// WARN WARN WARN: We won't have a final value for this until declaration | |||
/// diagnostics have been computed for all <see cref="SourceMemberContainerTypeSymbol"/>s, so pass | |||
/// ignoringInterfaceImplementationChanges: true if you need a value sooner | |||
/// option: <see cref="IsMetadataVirtualOption.IgnoreInterfaceImplementationChanges"/> true if you need a value sooner |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like a couple of tests in |
Done with review pass (commit 1) |
@dotnet/roslyn-compiler for second review. Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (commit 2)
@dotnet/roslyn-compiler for second review. Thanks |
@dotnet/roslyn-compiler for second review. Thanks |
Fixes #73563
IsMetadataVirtual
previously could either:ForceComplete
(which may initialize it inSourceMemberContainerTypeSymbolSynthesizeInterfaceMemberImplementation
)ignoreInterfaceImplementationChanges
)Previously, in a scenario where
comp2
referencescomp1
as a compilation reference and we emitcomp2
and then emitcomp1
:comp2
calledMethodToClassRewriter.VisitCall
which calledIsMetadataVirtual
on a not-yet-completed symbol fromcomp1
. The read fromIsMetadataVirtual
set the "locked" flag.comp1
completed that symbol fromcomp1
which would callEnsureMetadataVirtual
. This asserted since the "locked" flag was already setNow, the assertion in
MethodToClassRewriter.VisitCall
eagerly forces the completion of the symbol fromcomp1
.