Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/Compilers/CSharp/Portable/BoundTree/BoundMethodDefIndex.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Diagnostics;

namespace Microsoft.CodeAnalysis.CSharp
{
internal partial class BoundMethodDefIndex
{
private partial void Validate()
{
Debug.Assert(Method.IsDefinition);
}
}
}
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -739,7 +739,7 @@

<!-- Represents the raw metadata RowId value for a method definition.
Used by dynamic instrumentation to index into tables or arrays of per-method information. -->
<Node Name="BoundMethodDefIndex" Base="BoundExpression">
<Node Name="BoundMethodDefIndex" Base="BoundExpression" HasValidate="true">
<!-- Non-null type is required for this node kind -->
<Field Name="Type" Type="TypeSymbol" Override="true" Null="disallow"/>
<Field Name="Method" Type="MethodSymbol"/>
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,13 @@ public override BoundNode VisitAwaitableValuePlaceholder(BoundAwaitableValuePlac
;
}

public override BoundNode? VisitMethodDefIndex(BoundMethodDefIndex node)
{
// Cannot replace a MethodDefIndex's Method/Type with a substituted symbol.
Debug.Assert(node.Type.Equals(VisitType(node.Type), TypeCompareKind.ConsiderEverything));
return node;
Copy link
Member Author

@RikkiGibson RikkiGibson Jul 1, 2025

Choose a reason for hiding this comment

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

This fixes the observed issue and causes the new test to pass in debug mode.

In the absence of this change, we can end up using a substituted method symbol in EmitMethodDefIndexExpression. I didn't debug further past that, but, it looks like that causes bogus index values to be used into the instrumentation payloads.

In the linked issue and the repro unit test, it looks like the particular Method which was being substituted prior to this change, was the original member method AsAsyncEnumerable<T>, which had its type parameters substituted with that from the display class C.<AsAsyncEnumerable>d__0<T>.

}

[return: NotNullIfNotNull(nameof(method))]
public override MethodSymbol? VisitMethodSymbol(MethodSymbol? method)
{
Expand Down
254 changes: 254 additions & 0 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncIteratorTests.cs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace Microsoft.CodeAnalysis.CSharp.DynamicAnalysis.UnitTests
{
public class DynamicAnalysisResourceTests : CSharpTestBase
{
const string InstrumentationHelperSource = @"
public const string InstrumentationHelperSource = @"
namespace Microsoft.CodeAnalysis.Runtime
{
public static class Instrumentation
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
' Licensed to the .NET Foundation under one or more agreements.
' The .NET Foundation licenses this file to you under the MIT license.
' See the LICENSE file in the project root for more information.

Imports Microsoft.CodeAnalysis.Text
Imports Microsoft.CodeAnalysis.VisualBasic.Symbols
Imports Microsoft.CodeAnalysis.VisualBasic.Syntax

Namespace Microsoft.CodeAnalysis.VisualBasic
Partial Friend Class BoundMethodDefIndex
#If DEBUG Then
Private Sub Validate()
Debug.Assert(Method.IsDefinition)
End Sub
#End If
End Class
End Namespace
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@

<!-- Represents the raw metadata token index value for a method definition.
Used by dynamic instrumentation to index into tables or arrays of per-method information. -->
<Node Name="BoundMethodDefIndex" Base="BoundExpression">
<Node Name="BoundMethodDefIndex" Base="BoundExpression" HasValidate="true">
<!-- Non-null type is required for this node kind -->
<Field Name="Type" Type="TypeSymbol" Override="true" Null="disallow"/>
<Field Name="Method" Type="MethodSymbol"/>
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.