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

[Compat] Unhandled ArgumentException: operatorToken Parameter System.Half.explicit_operator checked byte (System.Half) #77101

Closed
carlossanlop opened this issue Feb 6, 2025 · 9 comments · Fixed by #77102

Comments

@carlossanlop
Copy link
Member

carlossanlop commented Feb 6, 2025

Describe the bug

@ericstj and I saw this bug earlier last week when testing GenAPI.

I am now seeing it being hit by the ApiDiff implementation too, as the code that hits the bug is shared by this tool as well.

To Reproduce

  1. Fetch the branch from this PR: [ApiDiff] Implementation of the new ApiDiff that reuses GenAPI features sdk#46425
  2. Build in release.
  3. Create two C# library projects, name them before and after.
  4. The before project should contain this:
namespace MyNamespace;
public readonly struct MyStruct
{
}
  1. The after project should contain this:
namespace MyNamespace;
public readonly struct MyStruct
{
    public static explicit operator byte(MyStruct value) => (byte)(MyStruct)value;
    public static explicit operator checked byte(MyStruct value) => checked((byte)(MyStruct)value);
}
  1. Build both projects.
  2. Run this command:
~\source\repos\sdk\artifacts\bin\Microsoft.DotNet.ApiDiff.Tool\Release\net8.0\Microsoft.DotNet.ApiDiff.Tool.exe \ 
  -o "~\source\repos\output\" \
  -b "~\source\repos\before\bin\Release\net10.0\" \
  -a "~\source\repos\after\bin\Release\net10.0\" \
  --tableOfContentsTitle "Whatever"

Exceptions (if any)

Output message:

Unhandled exception: System.ArgumentException: operatorToken (Parameter 'MyNamespace.MyStruct.explicit operator checked byte(MyNamespace.MyStruct)')
   at Microsoft.DotNet.GenAPI.SyntaxGeneratorExtensions.DeclarationExt(SyntaxGenerator syntaxGenerator, ISymbol symbol, ISymbolFilter symbolFilter) in C:\Users\calope\source\repos\sdk2\src\Compatibility\GenAPI\Microsoft.DotNet.GenAPI\SyntaxGeneratorExtensions.cs:line 108
   at Microsoft.DotNet.GenAPI.CSharpAssemblyDocumentGenerator.Visit(SyntaxNode namedTypeNode, INamedTypeSymbol namedType) in C:\Users\calope\source\repos\sdk2\src\Compatibility\GenAPI\Microsoft.DotNet.GenAPI\CSharpAssemblyDocumentGenerator.cs:line 243
   at Microsoft.DotNet.GenAPI.CSharpAssemblyDocumentGenerator.Visit(INamespaceSymbol namespaceSymbol) in C:\Users\calope\source\repos\sdk2\src\Compatibility\GenAPI\Microsoft.DotNet.GenAPI\CSharpAssemblyDocumentGenerator.cs:line 150
   at Microsoft.DotNet.GenAPI.CSharpAssemblyDocumentGenerator.GetDocumentForAssembly(IAssemblySymbol assemblySymbol) in C:\Users\calope\source\repos\sdk2\src\Compatibility\GenAPI\Microsoft.DotNet.GenAPI\CSharpAssemblyDocumentGenerator.cs:line 99
   at Microsoft.DotNet.ApiDiff.MemoryOutputDiffGenerator.GetAssemblyRootNodeAndModel(IAssemblySymbolLoader loader, IAssemblySymbol assemblySymbol) in C:\Users\calope\source\repos\sdk2\src\Compatibility\ApiDiff\Microsoft.DotNet.ApiDiff\MemoryOutputDiffGenerator.cs:line 362
   at Microsoft.DotNet.ApiDiff.MemoryOutputDiffGenerator.Run() in C:\Users\calope\source\repos\sdk2\src\Compatibility\ApiDiff\Microsoft.DotNet.ApiDiff\MemoryOutputDiffGenerator.cs:line 85
   at Microsoft.DotNet.ApiDiff.FileOutputDiffGenerator.Run() in C:\Users\calope\source\repos\sdk2\src\Compatibility\ApiDiff\Microsoft.DotNet.ApiDiff\FileOutputDiffGenerator.cs:line 106
   at Microsoft.DotNet.ApiDiff.Tool.Program.HandleCommand(DiffConfiguration diffConfig) in C:\Users\calope\source\repos\sdk2\src\Compatibility\ApiDiff\Microsoft.DotNet.ApiDiff.Tool\Program.cs:line 148
   at System.CommandLine.Handler.<>c__DisplayClass2_0`1.<SetHandler>b__0(InvocationContext context)
   at System.CommandLine.Invocation.AnonymousCommandHandler.Invoke(InvocationContext context)
   at System.CommandLine.Invocation.AnonymousCommandHandler.InvokeAsync(InvocationContext context)
   at System.CommandLine.Invocation.InvocationPipeline.<>c__DisplayClass4_0.<<BuildInvocationChain>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c__DisplayClass17_0.<<UseParseErrorReporting>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c__DisplayClass12_0.<<UseHelp>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c__DisplayClass22_0.<<UseVersionOption>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c__DisplayClass19_0.<<UseTypoCorrections>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c.<<UseSuggestDirective>b__18_0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c__DisplayClass16_0.<<UseParseDirective>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c.<<RegisterWithDotnetSuggest>b__5_0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.Builder.CommandLineBuilderExtensions.<>c__DisplayClass8_0.<<UseExceptionHandler>b__0>d.MoveNext()

Further technical details

  • Include the output of dotnet --info
~/source/repos/sdk2> .dotnet/dotnet.exe --info
.NET SDK:
 Version:           10.0.100-alpha.1.25077.2
 Commit:            23e2ba847d
 Workload version:  10.0.100-manifests.70ddf7f4
 MSBuild version:   17.14.0-preview-25073-02+291a81087

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.26100
 OS Platform: Windows
 RID:         win-x64
 Base Path:   C:\Users\calope\source\repos\sdk2\.dotnet\sdk\10.0.100-alpha.1.25077.2\

.NET workloads installed:
There are no installed workloads to display.
Configured to use loose manifests when installing new manifests.

Host:
  Version:      10.0.0-preview.2.25104.8
  Architecture: x64
  Commit:       9699929153

.NET SDKs installed:
  10.0.100-alpha.1.25077.2 [C:\Users\calope\source\repos\sdk2\.dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 10.0.0-alpha.2.24572.1 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 10.0.0-alpha.2.25056.9 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 10.0.0-alpha.2.25058.2 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 10.0.0-alpha.2.25061.1 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 10.0.0-alpha.2.25073.4 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 10.0.0-preview.2.25105.1 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.0 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.0 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.0 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 9.0.0 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 10.0.0-alpha.1.24570.9 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 10.0.0-alpha.1.25052.4 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 10.0.0-alpha.1.25057.17 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 10.0.0-alpha.1.25058.4 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 10.0.0-alpha.1.25058.25 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 10.0.0-alpha.1.25061.3 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 10.0.0-alpha.1.25062.3 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 10.0.0-alpha.1.25073.13 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 10.0.0-preview.2.25078.10 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 10.0.0-preview.2.25080.11 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 10.0.0-preview.2.25104.7 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 10.0.0-preview.2.25104.8 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 10.0.0-alpha.1.24572.3 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 10.0.0-alpha.1.25073.1 [C:\Users\calope\source\repos\sdk2\.dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
  x86   [C:\Program Files (x86)\dotnet]
    registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
  Not set

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download
  • The IDE (VS / VS Code/ VS4Mac) you're running on, and its version

I'm using VS Code.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Feb 6, 2025
Copy link
Contributor

@dotnet/area-infrastructure-libraries a new issue has been filed in the ApiCompat area, please triage

@carlossanlop
Copy link
Member Author

Got a simple repro using ApiDiff. See the description.

It seems we don't have any specialized if branch for a method of type "conversion". We only handle method of type "constructor". For everything else, we fallback to syntaxGenerator.Declaration(symbol), which is what's throwing the exception.

@carlossanlop
Copy link
Member Author

carlossanlop commented Feb 6, 2025

The sdk tools use net8.0 as target framework. I wonder if this is a case of a static explicit checked operator conversion not being available in that .NET version.

This is the code that throws:
https://github.com/dotnet/runtime/blob/fc8b63c3b36744b4012210e67cabb7fd96c938b5/src/libraries/System.Private.CoreLib/src/System/Half.cs#L811-L820

     public readonly struct Half
        : IComparable,
          ISpanFormattable,
          IComparable<Half>,
          IEquatable<Half>,
          IBinaryFloatingPointIeee754<Half>,
          IMinMaxValue<Half>,
          IUtf8SpanFormattable,
          IBinaryFloatParseAndFormatInfo<Half>
    {
        ...
        public static explicit operator byte(Half value) => (byte)(float)value;
        public static explicit operator checked byte(Half value) => checked((byte)(float)value);
        ...

This is where we should probably be handling a conversion method:
https://github.com/dotnet/sdk/blob/6b0ee2d6c4c1a6f39d47f10d781d6e9fab65a76c/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/SyntaxGeneratorExtensions.cs#L48

But we end up falling to the try catch here, then throwing:
https://github.com/dotnet/sdk/blob/6b0ee2d6c4c1a6f39d47f10d781d6e9fab65a76c/src/Compatibility/GenAPI/Microsoft.DotNet.GenAPI/SyntaxGeneratorExtensions.cs#L101-L109

@carlossanlop
Copy link
Member Author

I fed all runtime APIs to the tool and I found that the first checked operator that is analyzed by GenAPI is Half's public static explicit operator checked byte. Before that, I only found public static explicit operator byte (no checked).

@carlossanlop
Copy link
Member Author

@333fred was checked available in .NET 8, or was it introduced later?

@carlossanlop
Copy link
Member Author

The exception comes from this method when the operatorName string's value is "op_CheckedExplicit":

private static SyntaxKind GetOperatorSyntaxKind(string operatorName)
{
var operatorKind = CSharp.SyntaxFacts.GetOperatorKind(operatorName);
if (operatorKind == SyntaxKind.None)
{
throw new ArgumentException("Unknown operator kind.");

    private static SyntaxKind GetOperatorSyntaxKind(string operatorName)
    {
        var operatorKind = CSharp.SyntaxFacts.GetOperatorKind(operatorName);
        if (operatorKind == SyntaxKind.None)
        {
            throw new ArgumentException("Unknown operator kind.");

@ericstj
Copy link
Member

ericstj commented Feb 6, 2025

This isn't the TargetFramework -- roslyn brings with it the understanding of metadata and syntax. First we make sure we have the latest Roslyn. If we do, then it could be that SyntaxGenerator wasn't updated for this new syntax. We've found this a few times in the past. We can make a PR to roslyn to add support for the syntax if that's the case.

@333fred
Copy link
Member

333fred commented Feb 7, 2025

@333fred was checked available in .NET 8, or was it introduced later?

It was added in C# 11, .NET 7.0, but as Eric says, TFM isn't relevant for this type of question.

@ericstj
Copy link
Member

ericstj commented Feb 7, 2025

Yeah, I found the bug was in OperatorDeclarationSyntax.OperatorToken in Syntax.xml. Working on a fix @333fred can you transfer this to Roslyn?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants