Skip to content
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

Function pointer calling convention IDE support #59062

Closed
wants to merge 22 commits into from
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -5564,9 +5564,9 @@ await TestAsync(code,
Operators.Asterisk,
Keyword("unmanaged"),
Punctuation.OpenBracket,
Identifier("Stdcall"),
Class("Stdcall"),
Punctuation.Comma,
Identifier("SuppressGCTransition"),
Class("SuppressGCTransition"),
Punctuation.CloseBracket,
Punctuation.OpenAngle,
Keyword("int"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2189,5 +2189,69 @@ public async Task TestStringEscape(TestHost testHost)
Verbatim("\""),
Punctuation.Semicolon);
}

[Theory]
[CombinatorialData]
[WorkItem(59052, "https://github.com/dotnet/roslyn/issues/59052")]
public async Task FunctionPointerCallingConventions_ClassNotUsedByCompiler(TestHost testHost)
{
await TestAsync(@"
public unsafe class C
{
delegate* unmanaged[Stdcall]<void> f0;
}
",
testHost,
Keyword("public"),
Keyword("unsafe"),
Keyword("class"),
Class("C"),
Punctuation.OpenCurly,
Keyword("delegate"),
Operators.Asterisk,
Keyword("unmanaged"),
Punctuation.OpenBracket,
Class("Stdcall"),
Punctuation.CloseBracket,
Punctuation.OpenAngle,
Keyword("void"),
Punctuation.CloseAngle,
Field("f0"),
Punctuation.Semicolon,
Punctuation.CloseCurly);
}

[Theory]
[CombinatorialData]
[WorkItem(59052, "https://github.com/dotnet/roslyn/issues/59052")]
public async Task FunctionPointerCallingConventions_ClassIsUsedByCompiler(TestHost testHost)
{
await TestAsync(@"
public unsafe class C
{
delegate* unmanaged[Stdcall, Fastcall]<void> f0;
}
",
testHost,
Keyword("public"),
Keyword("unsafe"),
Keyword("class"),
Class("C"),
Punctuation.OpenCurly,
Keyword("delegate"),
Operators.Asterisk,
Keyword("unmanaged"),
Punctuation.OpenBracket,
Class("Stdcall"),
Punctuation.Comma,
Class("Fastcall"),
Punctuation.CloseBracket,
Punctuation.OpenAngle,
Keyword("void"),
Punctuation.CloseAngle,
Field("f0"),
Punctuation.Semicolon,
Punctuation.CloseCurly);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

using System;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CSharp.Completion.Providers;
Expand Down
115 changes: 115 additions & 0 deletions src/EditorFeatures/Test2/GoToDefinition/CSharpGoToDefinitionTests.vb
Original file line number Diff line number Diff line change
Expand Up @@ -3333,5 +3333,120 @@ $$

Test(workspace, expectedResult:=False)
End Sub

<WpfFact, Trait(Traits.Feature, Traits.Features.GoToDefinition)>
<WorkItem(59052, "https://github.com/dotnet/roslyn/issues/59052")>
Public Sub FunctionPointerCallingConvention_Single_SpecialConv()
Dim workspace =
<Workspace>
<Project Language="C#" CommonReferencesNet6="true">
<Document><![CDATA[
public unsafe class C
{
delegate* unmanaged[St$$dcall]<void> f0;
}
]]></Document>
</Project>
</Workspace>

Test(workspace)
End Sub

<WpfFact, Trait(Traits.Feature, Traits.Features.GoToDefinition)>
<WorkItem(59052, "https://github.com/dotnet/roslyn/issues/59052")>
Public Sub FunctionPointerCallingConvention_Single_NonSpecialConv()
Dim workspace =
<Workspace>
<Project Language="C#" CommonReferencesNet6="true">
<Document><![CDATA[
public unsafe class C
{
delegate* unmanaged[M$$emberFunction]<void> f0;
}
]]></Document>
</Project>
</Workspace>

Test(workspace)
End Sub

<WpfFact, Trait(Traits.Feature, Traits.Features.GoToDefinition)>
<WorkItem(59052, "https://github.com/dotnet/roslyn/issues/59052")>
Public Sub FunctionPointerCallingConvention_Multiple_SpecialConv()
Dim workspace =
<Workspace>
<Project Language="C#" CommonReferencesNet6="true">
<Document><![CDATA[
public unsafe class C
{
delegate* unmanaged[St$$dcall, MemberFunction]<void> f0;
}
]]></Document>
</Project>
</Workspace>

Test(workspace)
End Sub

<WpfFact, Trait(Traits.Feature, Traits.Features.GoToDefinition)>
<WorkItem(59052, "https://github.com/dotnet/roslyn/issues/59052")>
Public Sub FunctionPointerCallingConvention_Multiple_NonSpecialConv()
Dim workspace =
<Workspace>
<Project Language="C#" CommonReferencesNet6="true">
<Document><![CDATA[
public unsafe class C
{
delegate* unmanaged[Stdcall, M$$emberFunction]<void> f0;
}
]]></Document>
</Project>
</Workspace>

Test(workspace)
End Sub

<WpfFact, Trait(Traits.Feature, Traits.Features.GoToDefinition)>
<WorkItem(59052, "https://github.com/dotnet/roslyn/issues/59052")>
Public Sub FunctionPointerCallingConvention_Multiple_AllAreSpecialConv()
Dim workspace =
<Workspace>
<Project Language="C#" CommonReferencesNet6="true">
<Document><![CDATA[
public unsafe class C
{
delegate* unmanaged[St$$dcall, Fastcall]<void> f0;
}
]]></Document>
</Project>
</Workspace>

Test(workspace)
End Sub

<WpfFact, Trait(Traits.Feature, Traits.Features.GoToDefinition)>
<WorkItem(59052, "https://github.com/dotnet/roslyn/issues/59052")>
Public Sub FunctionPointerCallingConvention_NotFromCorLib()
Dim workspace =
<Workspace>
<Project Language="C#" CommonReferencesNet45="true">
<Document><![CDATA[
public unsafe class C
{
delegate* unmanaged[Stdcall, M$$emberFunction]<void> f0;
}

namespace System.Runtime.CompilerServices
{
public class CallConvMemberFunction
{
}
}
]]></Document>
</Project>
</Workspace>

Test(workspace, expectedResult:=False)
End Sub
End Class
End Namespace
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public partial class TestWorkspace
private const string CommonReferencesNet45AttributeName = "CommonReferencesNet45";
private const string CommonReferencesPortableAttributeName = "CommonReferencesPortable";
private const string CommonReferencesNetCoreAppName = "CommonReferencesNetCoreApp";
private const string CommonReferencesNet6Name = "CommonReferencesNet6";
private const string CommonReferencesNetStandard20Name = "CommonReferencesNetStandard20";
private const string ReferencesOnDiskAttributeName = "ReferencesOnDisk";
private const string FilePathAttributeName = "FilePath";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1078,6 +1078,14 @@ private static IList<MetadataReference> CreateCommonReferences(TestWorkspace wor
references = TargetFrameworkUtil.NetStandard20References.ToList();
}

var net6 = element.Attribute(CommonReferencesNet6Name);
if (net6 != null &&
((bool?)net6).HasValue &&
((bool?)net6).Value)
{
references = TargetFrameworkUtil.GetReferences(TargetFramework.Net60).ToList();
}

return references;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,10 @@ private static bool IsVerbatimStringToken(SyntaxToken token)
{
return ClassificationTypeNames.LabelName;
}
else if (token.Parent.IsKind(SyntaxKind.FunctionPointerUnmanagedCallingConvention))
{
return ClassificationTypeNames.ClassName;
Copy link
Member

Choose a reason for hiding this comment

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

don't we want to do this if GetCallingConventionSymbol returns non-null?

Copy link
Member

Choose a reason for hiding this comment

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

e.g. if you write delegate[Gibberish] we wouldn't classify that guy :)

Copy link
Member Author

Choose a reason for hiding this comment

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

@CyrusNajmabadi This matches the compiler classification :)

builder.Add(CreatePart(SymbolDisplayPartKind.ClassName, conventionType, conventionType.Name[CallConvLength..]));

Quick info relies on compiler classification too:

image

}
else
{
return ClassificationTypeNames.Identifier;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public CSharpSyntaxClassificationService(HostLanguageServices languageServices)
new OperatorOverloadSyntaxClassifier(),
new SyntaxTokenClassifier(),
new UsingDirectiveSyntaxClassifier(),
new DiscardSyntaxClassifier()
new DiscardSyntaxClassifier(),
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,22 @@ public ImmutableArray<ISymbol> GetBestOrAllSymbols(SemanticModel semanticModel,
{
AssignmentExpressionSyntax _ when token.Kind() == SyntaxKind.EqualsToken => GetDeconstructionAssignmentMethods(semanticModel, node).As<ISymbol>(),
ForEachVariableStatementSyntax _ when token.Kind() == SyntaxKind.InKeyword => GetDeconstructionForEachMethods(semanticModel, node).As<ISymbol>(),
FunctionPointerUnmanagedCallingConventionSyntax callingConvention => GetCallingConventionSymbol(semanticModel, callingConvention),
_ => GetSymbolInfo(semanticModel, node, token, cancellationToken).GetBestOrAllSymbols(),
};
}

private static ImmutableArray<ISymbol> GetCallingConventionSymbol(SemanticModel model, FunctionPointerUnmanagedCallingConventionSyntax syntax)
{
var type = model.Compilation.UnmanagedCallingConventionType(syntax.Name.ValueText);
if (type is null)
{
return ImmutableArray<ISymbol>.Empty;
}

return ImmutableArray.Create<ISymbol>(type);
}

private static SymbolInfo GetSymbolInfo(SemanticModel semanticModel, SyntaxNode node, SyntaxToken token, CancellationToken cancellationToken)
{
switch (node)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,5 +211,11 @@ public static ImmutableArray<IAssemblySymbol> GetReferencedAssemblySymbols(this

public static INamedTypeSymbol? DisallowNullAttribute(this Compilation compilation)
=> compilation.GetTypeByMetadataName(typeof(DisallowNullAttribute).FullName!);

public static INamedTypeSymbol? UnmanagedCallingConventionType(this Compilation compilation, string callConv)
{
var corLibrary = compilation.GetSpecialType(SpecialType.System_Object).ContainingAssembly;
Copy link
Member

Choose a reason for hiding this comment

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

❔ Is this a requirement?

Copy link
Member Author

Choose a reason for hiding this comment

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

@sharwell At compiler-side, yes.

specifierType = compilation.Assembly.CorLibrary.LookupTopLevelMetadataType(ref metadataName, digThroughForwardedTypes: false);

Copy link
Contributor

Choose a reason for hiding this comment

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

I remember @jkoritzinsky mentioning how this API could be particularly slow, so much so they tried to avoid using it entirely in the COM generators. Is that still a concern (and if so, should we consider using an alternative here), or has that been fixed now?

Copy link
Member Author

Choose a reason for hiding this comment

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

@Sergio0694 To my knowledge GetSpecialType is fast, and internally it should be calculated once, and then the lookup is a simple fast array index (not even a Dictionary<TKey, TValue>).

Not sure how it caused issues in generators. I'd love to know more about that.

Copy link
Member

Choose a reason for hiding this comment

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

There was a perf bug in Roslyn that made this call very slow a few months back. It's fixed now.

return corLibrary?.GetTypeByMetadataName("System.Runtime.CompilerServices.CallConv" + callConv);
}
}
}