dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.38k stars 4.75k forks source link

[Source Generator] Source Generator works with dotnet cli but fails with `TypeLoadException` in VS #108164

Closed LittleLittleCloud closed 1 month ago

LittleLittleCloud commented 2 months ago

Description

The source generator can successfully generate source code when I compile using dotnet cli. It also successfully generates source code in VS UI when I expand the analyzer node. But it raises a TypeLoadException when I compile and run my code in VS by hitting control + F5.

Here is the link to the source code of source generator: https://github.com/tryAGI/AutoSDK/tree/main/src/libs/AutoSDK.SourceGenerators

Here is the dotnet --list-sdks

8.0.400 [C:\Program Files\dotnet\sdk]
9.0.100-preview.7.24407.12 [C:\Program Files\dotnet\sdk]

Here is the error log in VS:

Build started at 9:24 AM...
1>------ Build started: Project: SourceGen, Configuration: Debug Any CPU ------
1>CSC : warning CS8784: Generator 'SdkGenerator' failed to initialize. It will not contribute to the output and compilation errors may occur as a result. Exception was of type 'TypeLoadException' with message 'Could not load type 'AutoSDK.Models.ModelData' from assembly 'AutoSDK, Version=0.0.0.0, Culture=neutral, PublicKeyToken=603f13207e65c17b'.'.
1>SourceGen -> C:\Users\xiaoyuz\source\repos\SourceGen\bin\Debug\net8.0\SourceGen.dll
1>Done building project "SourceGen.csproj".

Here is the screenshot of which VS I am using: image

Finally, the link to a minimal reproducable example for this bug.

Reproduction Steps

See above

Expected behavior

Source generator successfully generate source in VS

Actual behavior

It raise a TypeLoadException

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

huoyaoyuan commented 2 months ago

Questions for source generator belong to roslyn repo. Source generators must target netstandard2.0 since VS is still on netfx. It's expected to fail if you target net8.0. See https://github.com/dotnet/roslyn/discussions/72777

LittleLittleCloud commented 2 months ago

@huoyaoyuan Thanks for reply, I can transfer this issue to roslyn repo.

Curious to ask though, why the behavior is different when using source generator in dotnet cli versus using in VS. In the example above, the source generator can successfully generate code when I compile from command line using dotnet cli.

huoyaoyuan commented 2 months ago

Dotnet cli executes almost everything with the same .NET version of the SDK, while VS executes roslyn with .NET Framework because itself is still on .NET Framework.

Source generators or analyzers should target the exact same lowest framework version of roslyn, to ensure any process that's capable loading roslyn can load the analyzer/generator. It also should reference the lowest roslyn version it need to support.

jeffschwMSFT commented 2 months ago

@jaredpar can you help with the transfer to roslyn?

jaredpar commented 1 month ago

Analyzers and generators are plugins to the compiler and they need to operate wherever the compiler operates. That includes running on .NET Framework and NET Core (potentially multiple versions). That is why they are strongly recommended to target netstandard2.0. When you target net8.0 it's expected that you will fail in cases where the compiler runs on .NET Framework

LittleLittleCloud commented 1 month ago

@jaredpar Thanks for the explanation, can I understand it in the way that if my project targets to net 8.0 and source generator targets to netstandard 2.0, it's expected that the source generation might fail in Visual Studio if the source generator uses any API that is net 8.0 only?

jaredpar commented 1 month ago

it's expected that the source generation might fail in Visual Studio if the source generator uses any API that is net 8.0 only?

Correct. It will fail for any API that is net8.0.

HavenDV commented 1 month ago

This source generator does not use any net8.0 apis. It's usual netstandard2.0 source generator which uses netstandard 2.0 library inside.

The only possible problem I see is some duplicate files inside the analyzers directory: image image

P.S. Removed in latest dev version

huoyaoyuan commented 1 month ago

This source generator does not use any net8.0 apis.

The original message hints that its dependency (AutoSDK) attempts to load net8.0 api.

HavenDV commented 1 month ago

I don't see anything meaningful in the message except the version number. Another idea is that it might have something to do with using MinVer, there was a recent update to version 6.0.0 that might have broken something.

jaredpar commented 1 month ago

Think there was a bit of confusion here. My initial reading was that the generator targeted net8.0 which would be illegal. In fact it , and it's dependencies, target netstandard2.0 and are used in a net8.0 project which is just fine. Spent some time debugging this last night and this afternoon with @agocke and I believe this is a runtime issue.

Here is the build.rsp that I was using for the repro project. I removed a few red herrings like Microsoft.OpenApi.dll being passed twice as well as the net analyzers to get it down to the core problem. From there I was just running it against roslyn using effectively the following command line:

> dotnet exec C:\users\jaredpar\code\roslyn\artifacts\bin\csc\Debug\net7.0\csc.dll `@build.rsp

What we observed is the following:

  1. This exception occurs when running against the net7.0 roslyn
  2. This exception does not occur when running against the net8.0 roslyn
  3. In the repro case
    1. The AssemblyLoadContext.Load(AssemblyName) method is never called with Microsoft.OpenApi.dll (breakpoint never hit)
    2. The ETW for the repro shows that Microsoft.OpenApi.dll is loaded into our directory load context

ETW entry:

HasStack="True" ThreadID="23,796" ProcessorNumber="0" ClrInstanceID="9" AssemblyName="Microsoft.OpenApi, Version=1.6.21.0, Culture=neutral, PublicKeyToken=3f5743946376f042" Stage="AssemblyLoadContextLoad" AssemblyLoadContext=""DirectoryLoadContext 0" Microsoft.CodeAnalysis.AnalyzerAssemblyLoader+DirectoryLoadContext dotnet/roslyn#1" Result="Success" ResultAssemblyName="Microsoft.OpenApi, Version=1.6.21.0, Culture=neutral, PublicKeyToken=3f5743946376f042" ResultAssemblyPath="C:.tools.nuget\packages\autosdk.sourcegenerators\0.24.0\analyzers\dotnet\roslyn4.3.1\cs\Microsoft.OpenApi.dll" ErrorMessage="" ActivityID="/#27016/1/44/"

This appears to be a runtime issue but thus far I can only reproduce it in net7.0. Sending back to runtime team for evaluation. This runtime is out of service but possible that the bug still exists in net8.0 and I'm just unable to hit it for some unknown reason. The customer report only has the 8.0 runtime so it indicates that it's likely still a bug there.

@agocke, @elinor-fung FYI

dotnet-policy-service[bot] commented 1 month ago

Tagging subscribers to this area: @vitek-karas, @agocke, @vsadov See info in area-owners.md if you want to be subscribed.

elinor-fung commented 1 month ago

I think assembly loading / Microsoft.OpenAPI.dll is a red herring here. When we hit the TypeLoadException thrown for AutoSDK.Models.ModelData, the relevant assemblies are already found/loaded as expected.

With the repro project, I could repro the TypeLoadException in:

In .NET 7, we are detecting a recursive type load and throwing: https://github.com/dotnet/runtime/blob/0fb6ac59fb1edbe4ed3ad62661df0eb7eacd7903/src/coreclr/vm/clsload.cpp#L3448-L3452

This looks like https://github.com/dotnet/runtime/issues/6924, which @davidwrighton fixed in .NET 8 with https://github.com/dotnet/runtime/pull/83995.

AutoSDK.Models.ModelData has a Parents property that is a generic of ModelData itself - ImmutableArray<ModelData>, which would result in hitting #6924.

I expect the VS build is hitting the same problem - assuming this is a long-standing issue that has always existed in .NET Framework. And building from the command line is fine because it is using .NET 8.

@davidwrighton does that sound right?

davidwrighton commented 1 month ago

Yes @elinor-fung appears to have the correct analysis. This is a bug that has been fixed in the runtime, but the fix is far too risky to backport to the .NET Framework.

elinor-fung commented 1 month ago

I think this goes back to @jaredpar's comment at https://github.com/dotnet/runtime/issues/108164#issuecomment-2369650801 then:

Analyzers and generators are plugins to the compiler and they need to operate wherever the compiler operates. That includes running on .NET Framework and NET Core (potentially multiple versions).

In this case, even though the generator is targeting netstandard2.0 and not using net8.0 APIs, it is trying to do something that is only supported in .NET 8 and later - load a type with a self-referencing generic, so it will fail in all the scenarios where the compiler operates on .NET Framework (like building in VS) or < .NET 8.