Demonstrate issue with GetSymbolInfo and function pointers. #81135

CyrusNajmabadi · 2025-11-10T17:23:11Z

No description provided.

CyrusNajmabadi · 2025-11-10T17:25:57Z

@333fred i was debugging through #81113

The problem on the IDE side is that we try to call GetSymbolInfo on C.M and we get back a symbolinfo that says 'OverloadResolutionFailed' and only inccludes C.M() in the .CandidateSymbols not the .Symbol. This causes us to bail out as we don't want to simplify something in error.

Can you ptal here? It seems odd that the code is entirely correct and compiles without error, but we get back a wonky SymbolInfo like this.

I've included a debuggable test here for you in FunctionPointerTests. Thanks!

333fred

What is the actual issue here?

333fred · 2025-11-10T17:26:11Z

src/Compilers/CSharp/Test/Semantic/Semantics/FunctionPointerTests.cs

                );
        }
+
+        [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75933")]


How is this related to this issue?

333fred · 2025-11-10T17:26:30Z

src/Compilers/CSharp/Test/Semantic/Semantics/FunctionPointerTests.cs

+            var memberAccess = root.DescendantNodes().OfType<MemberAccessExpressionSyntax>().Single();
+            var symbolInfo = semanticModel.GetSymbolInfo(memberAccess);
+
+            Assert.NotNull(symbolInfo.Symbol);


Please assert ToTestDisplayString, not just not null. #Resolved

…t cast expression for bindable parent node.

jcouv · 2025-11-11T06:41:09Z

src/Compilers/CSharp/Test/Semantic/Semantics/FunctionPointerTests.cs

+        }
+
+        [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/81113")]
+        public void TestGetSymbolInfo_Parethesized()


There's a typo in test name. Also the test doesn't seem useful as parens generally do nothing. I'd remove this test

The test is intentional to make sure we are walking through parentheses.

Also, I don't know what typo you are referring to?

Parethesized => Parenthesized

I think the point is exactly that things work properly even in the presence of parens. We don't want a set of parens making it so that semantic info breaks again.

jcouv · 2025-11-11T06:48:22Z

src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs

                        }
                        break;

+                    case SyntaxKind.AddressOfExpression:


I didn't understand the fix.
The method says "some nodes don't have direct semantic meaning by themselves and so we need to bind a different node that does"
But I don't follow why addressOf would fall in that category.
Also, it is strange that addressOf would be the only expression that needs special treatment for casts here.

If we bind the addressOf, what is wrong with the resulting bound node? (you could share .Dump() info from that node to illustrate the problem)

As the comment says, when the bound tree involves an address of being the direct object of a cast, there is no boundnode for the address of in the tree at all. You have a BoundConversion, representing the (delegate*<void>)&C.M, and it's operand, a BoundMethodGroup representing C.M. Since there is no node in the tree for the &C.M itself, GetBoundNodes will do a fallback "there must be an error here" binding (

roslyn/src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs

Lines 2177 to 2191 in b28fc31

if (results.IsEmpty)

{

// https://github.com/dotnet/roslyn/issues/35038: We have to run analysis on this node in some manner

using (_nodeMapLock.DisposableWrite())

{

var boundNode = this.Bind(incrementalBinder, node, BindingDiagnosticBag.Discarded);

GuardedAddBoundTreeForStandaloneSyntax(node, boundNode);

results = GuardedGetBoundNodesFromMap(node);

}

if (!results.IsEmpty)

{

return results;

}

}

), resulting in a BoundUnconvertedAddressOf node that is marked as failing overload resolution.

Thanks. Your explanation was clearer than comment :-P

jcouv · 2025-11-11T06:49:34Z

There seems to be legitimate CI test failures

Does this PR close issue #81113 ?

src/Features/CSharpTest/SimplifyTypeNames/SimplifyTypeNamesTests.cs

…ts.cs

src/Compilers/CSharp/Test/Semantic/Semantics/FunctionPointerTests.cs

jcouv

LGTM Thanks (commit 5) modulo typo

CyrusNajmabadi · 2025-11-11T13:44:07Z

i can't approve the PR, even though i didn't write the fix.

Going to take another approach

CyrusNajmabadi · 2025-11-12T16:47:56Z

@333fred seems like we have some failures in tests like:

Microsoft.CodeAnalysis.CSharp.UnitTests.CodeGen.CodeGenFunctionPointersTests.AddressOf_CannotAssignToVoidStar

333fred · 2025-11-12T21:37:49Z

Yes, I'm aware. This is going to need to be a bigger change to the bound tree.

…sts.cs Co-authored-by: Julien Couvreur <[email protected]>

CyrusNajmabadi added 2 commits November 10, 2025 18:19

Add test

0f51855

Failing teest

315dcd5

CyrusNajmabadi requested review from a team as code owners November 10, 2025 17:23

github-actions bot added the Area-Compilers label Nov 10, 2025

dotnet-policy-service bot added the VSCode label Nov 10, 2025

CyrusNajmabadi requested a review from 333fred November 10, 2025 17:24

CyrusNajmabadi assigned 333fred Nov 10, 2025

333fred reviewed Nov 10, 2025

View reviewed changes

CyrusNajmabadi and others added 2 commits November 10, 2025 18:36

Update test

d09d6ef

When the direct parent of an address of is a cast expression, use tha…

1384efa

…t cast expression for bindable parent node.

jcouv reviewed Nov 11, 2025

View reviewed changes

jcouv self-assigned this Nov 11, 2025

CyrusNajmabadi commented Nov 11, 2025

View reviewed changes

src/Features/CSharpTest/SimplifyTypeNames/SimplifyTypeNamesTests.cs Outdated Show resolved Hide resolved

Update src/Features/CSharpTest/SimplifyTypeNames/SimplifyTypeNamesTes…

45cdd2a

…ts.cs

jcouv reviewed Nov 11, 2025

View reviewed changes

src/Compilers/CSharp/Test/Semantic/Semantics/FunctionPointerTests.cs Outdated Show resolved Hide resolved

jcouv previously approved these changes Nov 11, 2025

View reviewed changes

333fred marked this pull request as draft November 11, 2025 22:46

This was referenced Nov 12, 2025

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

[android-arm64] The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#6408

Open

CyrusNajmabadi added 2 commits November 12, 2025 17:39

Merge remote-tracking branch 'upstream/main' into simplifyMemberAccess

6a98b83

Update test

8b79472

Update src/Compilers/CSharp/Test/Semantic/Semantics/FunctionPointerTe…

e2cfde5

…sts.cs Co-authored-by: Julien Couvreur <[email protected]>

	if (results.IsEmpty)
	{
	// https://github.com/dotnet/roslyn/issues/35038: We have to run analysis on this node in some manner
	using (_nodeMapLock.DisposableWrite())
	{
	var boundNode = this.Bind(incrementalBinder, node, BindingDiagnosticBag.Discarded);
	GuardedAddBoundTreeForStandaloneSyntax(node, boundNode);
	results = GuardedGetBoundNodesFromMap(node);
	}

	if (!results.IsEmpty)
	{
	return results;
	}
	}

Demonstrate issue with GetSymbolInfo and function pointers. #81135

Are you sure you want to change the base?

Demonstrate issue with GetSymbolInfo and function pointers. #81135

Conversation

CyrusNajmabadi commented Nov 10, 2025

Uh oh!

CyrusNajmabadi commented Nov 10, 2025

Uh oh!

333fred left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

333fred Nov 10, 2025 • edited by jcouv Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcouv commented Nov 11, 2025

Uh oh!

Uh oh!

Uh oh!

jcouv left a comment

Choose a reason for hiding this comment

Uh oh!

CyrusNajmabadi commented Nov 11, 2025

Uh oh!

CyrusNajmabadi commented Nov 12, 2025

Uh oh!

333fred commented Nov 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

333fred Nov 10, 2025 •

edited by jcouv

Loading