-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Do not use a constructed method symbol in a BoundMethodDefIndex #79211
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
Changes from all commits
79c6102
1b8c015
1d034cf
9104fbf
0ea6724
130c55e
030516e
6191e1a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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); | ||
| } | ||
| } | ||
| } |
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 |
|---|---|---|
|
|
@@ -188,6 +188,13 @@ public override BoundNode VisitAwaitableValuePlaceholder(BoundAwaitableValuePlac | |
| ; | ||
| } | ||
|
|
||
| public override BoundNode? VisitMethodDefIndex(BoundMethodDefIndex node) | ||
RikkiGibson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| // Cannot replace a MethodDefIndex's Method/Type with a substituted symbol. | ||
| Debug.Assert(node.Type.Equals(VisitType(node.Type), TypeCompareKind.ConsiderEverything)); | ||
| return node; | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 In the linked issue and the repro unit test, it looks like the particular
RikkiGibson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| [return: NotNullIfNotNull(nameof(method))] | ||
| public override MethodSymbol? VisitMethodSymbol(MethodSymbol? method) | ||
| { | ||
|
|
||
Large diffs are not rendered by default.
| 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 |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.