Adjust IsMetadataVirtual assertion in MethodToClassRewriter (#75066)

src/Compilers/CSharp/Portable/Emitter/Model/MethodSymbolAdapter.cs

@@ -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"/> if you need a value sooner
         /// and aren't concerned about tweaks made to satisfy interface implementation 
         /// requirements.
         /// NOTE: Not ignoring changes can only result in a value that is more true.
+        ///
+        /// Use IsMetadataVirtualOption.ForceCompleteIfNeeded in DEBUG/assertion code
+        /// to get the final value.
         /// </summary>
-        internal abstract bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false);
+        internal abstract bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None);
+
+        internal enum IsMetadataVirtualOption
+        {
+            None = 0,
+            IgnoreInterfaceImplementationChanges,
+#if DEBUG
+            ForceCompleteIfNeeded
+#endif
+        }
 
         internal virtual bool ReturnValueIsMarshalledExplicitly
         {

src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorFinallyMethodSymbol.cs

@@ -54,7 +54,7 @@ internal override bool IsMetadataNewSlot(bool ignoreInterfaceImplementationChang
             return false;
         }
 
-        internal override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+        internal override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
         {
             return false;
         }

src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs

@@ -233,7 +233,11 @@ public override BoundNode VisitCall(BoundCall node)
             var rewrittenArguments = (ImmutableArray<BoundExpression>)this.VisitList(node.Arguments);
             var rewrittenType = this.VisitType(node.Type);
 
-            Debug.Assert(rewrittenMethodSymbol.IsMetadataVirtual() == node.Method.IsMetadataVirtual());
+#if DEBUG
+            // Calling IsMetadataVirtual with intent of getting a fully accurate result requires the symbol to be fully completed first
+            Debug.Assert(rewrittenMethodSymbol.IsMetadataVirtual(MethodSymbol.IsMetadataVirtualOption.ForceCompleteIfNeeded)
+                == node.Method.IsMetadataVirtual(MethodSymbol.IsMetadataVirtualOption.ForceCompleteIfNeeded));
+#endif
 
             // If the original receiver was a base access and it was rewritten, 
             // change the method to point to the wrapper method

src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.ConstructorSymbol.cs

@@ -70,7 +70,7 @@ public override bool IsOverride
                 get { return false; }
             }
 
-            internal sealed override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+            internal sealed override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
             {
                 return false;
             }

src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.EqualsMethodSymbol.cs

@@ -55,7 +55,7 @@ public override bool IsOverride
                 get { return true; }
             }
 
-            internal sealed override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+            internal sealed override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
             {
                 return true;
             }

src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.GetHashCodeMethodSymbol.cs

@@ -59,7 +59,7 @@ public override bool IsOverride
                 get { return true; }
             }
 
-            internal sealed override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+            internal sealed override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
             {
                 return true;
             }

src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.PropertyAccessorSymbol.cs

@@ -70,7 +70,7 @@ public override bool IsOverride
                 get { return false; }
             }
 
-            internal sealed override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+            internal sealed override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
             {
                 return false;
             }

src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.ToStringMethodSymbol.cs

@@ -59,7 +59,7 @@ public override bool IsOverride
                 get { return true; }
             }
 
-            internal sealed override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+            internal sealed override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
             {
                 return true;
             }

src/Compilers/CSharp/Portable/Symbols/ErrorMethodSymbol.cs

@@ -216,7 +216,7 @@ internal sealed override bool IsMetadataNewSlot(bool ignoreInterfaceImplementati
             return false;
         }
 
-        internal sealed override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+        internal sealed override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
         {
             return false;
         }

src/Compilers/CSharp/Portable/Symbols/FunctionPointers/FunctionPointerMethodSymbol.cs

@@ -846,7 +846,7 @@ public override bool IsVararg
         public override ImmutableHashSet<string> ReturnNotNullIfParameterNotNull => ImmutableHashSet<string>.Empty;
         public override FlowAnalysisAnnotations FlowAnalysisAnnotations => FlowAnalysisAnnotations.None;
         internal override bool IsMetadataNewSlot(bool ignoreInterfaceImplementationChanges = false) => false;
-        internal override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false) => false;
+        internal override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None) => false;
         internal sealed override UnmanagedCallersOnlyAttributeData? GetUnmanagedCallersOnlyAttributeData(bool forceComplete) => null;
 
         internal override bool GenerateDebugInfo => throw ExceptionUtilities.Unreachable();

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs

@@ -595,7 +595,7 @@ public override int Arity
 
         public override bool IsStatic => HasFlag(MethodAttributes.Static);
 
-        internal override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false) => HasFlag(MethodAttributes.Virtual);
+        internal override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None) => HasFlag(MethodAttributes.Virtual);
 
         internal override bool IsMetadataNewSlot(bool ignoreInterfaceImplementationChanges = false) => HasFlag(MethodAttributes.NewSlot);
 

src/Compilers/CSharp/Portable/Symbols/MethodSymbolExtensions.cs

@@ -43,7 +43,7 @@ public static bool IsRuntimeFinalizer(this MethodSymbol method, bool skipFirstMe
             // IsMetadataVirtualIgnoringInterfaceImplementationChanges.  This also has the advantage of making
             // this method safe to call before declaration diagnostics have been computed.
             if ((object)method == null || method.Name != WellKnownMemberNames.DestructorName ||
-                method.ParameterCount != 0 || method.Arity != 0 || !method.IsMetadataVirtual(ignoreInterfaceImplementationChanges: true))
+                method.ParameterCount != 0 || method.Arity != 0 || !method.IsMetadataVirtual(MethodSymbol.IsMetadataVirtualOption.IgnoreInterfaceImplementationChanges))
             {
                 return false;
             }

src/Compilers/CSharp/Portable/Symbols/OverriddenOrHiddenMembersHelpers.cs

@@ -1016,7 +1016,7 @@ internal static MethodSymbol GetFirstRuntimeOverriddenMethodIgnoringNewSlot(this
             // be computed, then it is important for us to pass ignoreInterfaceImplementationChanges: true
             // (see MethodSymbol.IsMetadataVirtual for details).
             // Since we are only concerned with overrides (of class methods), interface implementations can be ignored.
-            const bool ignoreInterfaceImplementationChanges = true;
+            const MethodSymbol.IsMetadataVirtualOption ignoreInterfaceImplementationChanges = MethodSymbol.IsMetadataVirtualOption.IgnoreInterfaceImplementationChanges;
 
             wasAmbiguous = false;
             if (!method.IsMetadataVirtual(ignoreInterfaceImplementationChanges) || method.IsStatic)

src/Compilers/CSharp/Portable/Symbols/ReducedExtensionMethodSymbol.cs

@@ -423,7 +423,7 @@ internal sealed override bool IsMetadataNewSlot(bool ignoreInterfaceImplementati
             return false;
         }
 
-        internal sealed override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+        internal sealed override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
         {
             return false;
         }

src/Compilers/CSharp/Portable/Symbols/SignatureOnlyMethodSymbol.cs

@@ -162,7 +162,7 @@ internal sealed override bool HasAsyncMethodBuilderAttribute(out TypeSymbol buil
 
         internal sealed override bool IsMetadataNewSlot(bool ignoreInterfaceImplementationChanges = false) { throw ExceptionUtilities.Unreachable(); }
 
-        internal sealed override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false) { throw ExceptionUtilities.Unreachable(); }
+        internal sealed override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None) { throw ExceptionUtilities.Unreachable(); }
 
         internal override bool IsMetadataFinal
         {

src/Compilers/CSharp/Portable/Symbols/Source/LambdaSymbol.cs

@@ -114,7 +114,7 @@ internal sealed override bool IsMetadataNewSlot(bool ignoreInterfaceImplementati
             return false;
         }
 
-        internal sealed override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+        internal sealed override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
         {
             return false;
         }

src/Compilers/CSharp/Portable/Symbols/Source/LocalFunctionSymbol.cs

@@ -378,7 +378,7 @@ protected override void NoteAttributesComplete(bool forReturnType) { }
 
         internal override bool IsMetadataNewSlot(bool ignoreInterfaceImplementationChanges = false) => false;
 
-        internal override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false) => false;
+        internal override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None) => false;
 
         internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree localTree)
         {

src/Compilers/CSharp/Portable/Symbols/Source/SourceDestructorSymbol.cs

@@ -154,7 +154,7 @@ internal override OneOrMany<SyntaxList<AttributeListSyntax>> GetReturnTypeAttrib
             return OneOrMany.Create(default(SyntaxList<AttributeListSyntax>));
         }
 
-        internal sealed override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+        internal sealed override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
         {
             return true;
         }

src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol_ImplementationChecks.cs

@@ -1838,7 +1838,7 @@ private void CheckInterfaceUnification(BindingDiagnosticBag diagnostics)
                             needSynthesizedImplementation = false;
                         }
                     }
-                    else if (implementingMethod.IsMetadataVirtual(ignoreInterfaceImplementationChanges: true))
+                    else if (implementingMethod.IsMetadataVirtual(MethodSymbol.IsMetadataVirtualOption.IgnoreInterfaceImplementationChanges))
                     {
                         // If the signatures match and the implementation method is definitely virtual, then we're set.
                         needSynthesizedImplementation = false;

src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberMethodSymbol.cs

@@ -554,13 +554,20 @@ internal override bool IsMetadataNewSlot(bool ignoreInterfaceImplementationChang
             // in NamedTypeSymbolAdapter.cs).
             return this.IsOverride ?
                 this.RequiresExplicitOverride(out _) :
-                !this.IsStatic && this.IsMetadataVirtual(ignoreInterfaceImplementationChanges);
+                !this.IsStatic && this.IsMetadataVirtual(ignoreInterfaceImplementationChanges ? IsMetadataVirtualOption.IgnoreInterfaceImplementationChanges : IsMetadataVirtualOption.None);
         }
 
         // TODO (tomat): sealed?
-        internal override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+        internal override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
         {
-            return this.flags.IsMetadataVirtual(ignoreInterfaceImplementationChanges);
+#if DEBUG
+            if (option == IsMetadataVirtualOption.ForceCompleteIfNeeded && !this.flags.IsMetadataVirtualLocked)
+            {
+                this.ContainingSymbol.ForceComplete(locationOpt: null, filter: null, cancellationToken: CancellationToken.None);
+            }
+#endif
+
+            return this.flags.IsMetadataVirtual(ignoreInterfaceImplementationChanges: option == IsMetadataVirtualOption.IgnoreInterfaceImplementationChanges);
         }
 
         internal void EnsureMetadataVirtual()

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedDelegateSymbol.cs

@@ -77,7 +77,7 @@ internal override bool IsMetadataNewSlot(bool ignoreInterfaceImplementationChang
             return true;
         }
 
-        internal override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+        internal override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
         {
             return true;
         }

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEntryPointSymbol.cs

@@ -229,7 +229,7 @@ internal sealed override bool IsMetadataNewSlot(bool ignoreInterfaceImplementati
             return false;
         }
 
-        internal sealed override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+        internal sealed override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
         {
             return false;
         }

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedGlobalMethodSymbol.cs

@@ -291,7 +291,7 @@ internal sealed override bool IsMetadataNewSlot(bool ignoreInterfaceImplementati
             return false;
         }
 
-        internal sealed override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+        internal sealed override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
         {
             return false;
         }

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedImplementationMethod.cs

@@ -223,7 +223,7 @@ internal sealed override bool RequiresSecurityObject
             get { return _interfaceMethod.RequiresSecurityObject; }
         }
 
-        internal sealed override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+        internal sealed override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
         {
             return !IsStatic;
         }

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedInstanceConstructor.cs

@@ -240,7 +240,7 @@ internal sealed override bool IsMetadataNewSlot(bool ignoreInterfaceImplementati
             return false;
         }
 
-        internal sealed override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+        internal sealed override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
         {
             return false;
         }

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedInteractiveInitializerMethod.cs

@@ -225,7 +225,7 @@ internal override bool IsMetadataNewSlot(bool ignoreInterfaceImplementationChang
             return false;
         }
 
-        internal override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+        internal override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
         {
             return false;
         }

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedIntrinsicOperatorSymbol.cs

@@ -97,7 +97,7 @@ internal override bool IsMetadataNewSlot(bool ignoreInterfaceImplementationChang
             return false;
         }
 
-        internal override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+        internal override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
         {
             return false;
         }

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedSealedPropertyAccessor.cs

@@ -307,7 +307,7 @@ internal override System.Reflection.MethodImplAttributes ImplementationAttribute
             }
         }
 
-        internal sealed override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+        internal sealed override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
         {
             return true;
         }

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedStaticConstructor.cs

@@ -314,7 +314,7 @@ internal sealed override bool IsMetadataNewSlot(bool ignoreInterfaceImplementati
             return false;
         }
 
-        internal sealed override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+        internal sealed override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
         {
             return false;
         }

src/Compilers/CSharp/Portable/Symbols/Wrapped/WrappedMethodSymbol.cs

@@ -186,9 +186,9 @@ public override bool IsImplicitlyDeclared
             }
         }
 
-        internal override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
+        internal override bool IsMetadataVirtual(IsMetadataVirtualOption option = IsMetadataVirtualOption.None)
         {
-            return UnderlyingMethod.IsMetadataVirtual(ignoreInterfaceImplementationChanges);
+            return UnderlyingMethod.IsMetadataVirtual(option);
         }
 
         internal override bool IsMetadataFinal

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncIteratorTests.cs

@@ -8585,5 +8585,86 @@ async Task<int> Do()
             comp.VerifyEmitDiagnostics();
             CompileAndVerify(comp, expectedOutput: ExpectedOutput("42"), verify: Verification.FailsPEVerify);
         }
+
+        [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/73563")]
+        public void IsMetadataVirtual_01()
+        {
+            var src1 = @"
+using System.Collections.Generic;
+using System.Threading;
+
+public struct S : IAsyncEnumerable<int>
+{
+    public IAsyncEnumerator<int> GetAsyncEnumerator(CancellationToken token = default) => throw null;
+
+    void M()
+    {
+        GetAsyncEnumerator();
+    }
+}
+";
+
+            var comp1 = CreateCompilation(src1, targetFramework: TargetFramework.Net80);
+
+            var src2 = @"
+using System.Threading.Tasks;
+
+class C
+{
+    static async Task Main()
+    {
+        await foreach (var i in new S())
+        {
+        }
+    }
+}
+";
+            var comp2 = CreateCompilation(src2, references: [comp1.ToMetadataReference()], targetFramework: TargetFramework.Net80);
+            comp2.VerifyEmitDiagnostics(); // Indirectly calling IsMetadataVirtual on S.GetAsyncEnumerator (a read which causes the lock to be set)
+            comp1.VerifyEmitDiagnostics(); // Would call EnsureMetadataVirtual on S.GetAsyncEnumerator and would therefore assert if S was not already ForceCompleted
+        }
+
+        [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/73563")]
+        public void IsMetadataVirtual_02()
+        {
+            var src1 = @"
+using System;
+using System.Threading.Tasks;
+
+public struct S2 : IAsyncDisposable
+{
+    public ValueTask DisposeAsync()
+    {
+        return ValueTask.CompletedTask;
+    }
+
+    void M()
+    {
+        DisposeAsync();
+    }
+}
+";
+
+            var comp1 = CreateCompilation(src1, targetFramework: TargetFramework.Net80);
+
+            var src2 = @"
+class C
+{
+    static async System.Threading.Tasks.Task Main()
+    {
+        await using (new S2())
+        {
+        }
+
+        await using (var s = new S2())
+        {
+        }
+    }
+}
+";
+            var comp2 = CreateCompilation(src2, references: [comp1.ToMetadataReference()], targetFramework: TargetFramework.Net80);
+            comp2.VerifyEmitDiagnostics(); // Indirectly calling IsMetadataVirtual on S.DisposeAsync (a read which causes the lock to be set)
+            comp1.VerifyEmitDiagnostics(); // Would call EnsureMetadataVirtual on S.DisposeAsync and would therefore assert if S was not already ForceCompleted
+        }
     }
 }