roslyn: SourceGenerators in shared compilation can cause MissingMethodException

Version Used: Visual Studio v17.5p1 Roslyn in .NET 7.0.101 and in .NETFramework 4.7.2 (the one that ships with VS)

Steps to Reproduce:

> mkdir CTRepro
> cd CTRepro
> dotnet new console
> dotnet add package CommunityToolkit.Mvvm -v 8.1.0-preview2
> dotnet add package Microsoft.Windows.CsWin32 -v 0.2.164-beta

then add a new .cs file so the CommunityToolkit SourceGenerators will run

using CommunityToolkit.Mvvm.Input;

namespace CTRepro;

internal partial class Model
{
    [RelayCommand]
    private void Execute() { }
}

Then you can either build in VS or use msbuild -restore /t:Rebuild to trigger the warning

CSC : warning CS8785: Generator 'RelayCommandGenerator' failed to generate source. It will not contribute to the output and compilation errors may occur as a result. Exception was of type 'MissingMethodException' with message 'Method not found: 'System.ReadOnlySpan`1<!0> System.Collections.Immutable.ImmutableArray`1.AsSpan()'.' [C:\Users\hank.mccord\source\repos\CTRepro\CTRepro.csproj]

Expected Behavior:

The generated code should include the ICommand that is generated

Actual Behavior:

Because the source generator encountered an error, the associated IL is missing entirely. If you build with dotnet build, it works as expected.

Investigation/Cause

A couple things collide to cause this:

Using msbuild or Visual Studio uses the full framework MSBuild (i.e. no ALCs) under a shared compilation context
CsWin32 is shipping System.Memory in their analyzers directory
VBCSCompiler eventually loads System.Memory in the LoadFrom context (even though System.Memory is in the probing path)

I hope this enough information. Someone who is very familiar with assembly loading process should get the gist of what’s going on.
The .NET version of roslyn has the names of simple assemblies that will be provided in by the compiler but the .NETFramework has a completely different loading process since there are no ALCs.

Is this a concern to the Roslyn team or should the fix be CsWin32 package not shipping Roslyn provided dlls in their analyzer folder?

[Update(jcouv):] This issue is referenced by some skipped tests.

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 22 (20 by maintainers)

Commits related to this issue

Fix analyzer loading to prefer host This changes the design of our analyzer loading policy to give hosts more control over dependencies. Whenever possible analyzer dependencies will be unified with t... — committed to jaredpar/roslyn by jaredpar a year ago
Fix analyzer loading to prefer host This changes the design of our analyzer loading policy to give hosts more control over dependencies. Whenever possible analyzer dependencies will be unified with t... — committed to jaredpar/roslyn by jaredpar a year ago
Fix analyzer loading to prefer host This changes the design of our analyzer loading policy to give hosts more control over dependencies. Whenever possible analyzer dependencies will be unified with t... — committed to jaredpar/roslyn by jaredpar a year ago
Fix analyzer loading to prefer host This changes the design of our analyzer loading policy to give hosts more control over dependencies. Whenever possible analyzer dependencies will be unified with t... — committed to jaredpar/roslyn by jaredpar a year ago
Fix analyzer loading to prefer host (#66492) * Fix analyzer loading to prefer host This changes the design of our analyzer loading policy to give hosts more control over dependencies. Whenever po... — committed to dotnet/roslyn by deleted user a year ago

Most upvoted comments

When csc.exe’s only world is a .NET world (no .NET Framework) I believe you’ll be able to do a much better job of getting this right. Or at least, it will be a simpler problem.

I just prepared a CsWin32 change to drop all dependencies that msbuild/csc ships. I tested the change in VS Code (omnisharp) and Rider (started my 30 day ticking timer on a trial to do it) and it works everywhere. I can’s speak to older versions of all these, but given it works, and it sounds like it’ll solve the MVVM Toolkit problem, I’m happy to merge this.

AArnott on Jan 6, 2023

.NETFramework is the one with load contexts.

So you’re saying .NET Framework has a concept of load contexts…for loading assemblies… which is entirely unrelated to .NET Core’s concept of assembly load contexts? 🙁

https://learn.microsoft.com/en-us/dotnet/framework/deployment/best-practices-for-assembly-loading

RikkiGibson on Jan 5, 2023

I do have a fix for this behavior but it was more involved than I originally thought. Had to break it up.

jaredpar on Jan 11, 2023

I would argue though, that it’s not an automatic “bug” to ship a dll that the compiler also ships.

I should’ve been clearer in my response about this. It intuitively feels wrong to me to do this and I deeply wish I could proclaim it a bug and wash my hands of it. But the reality of the situation is complex, particularly when analyzers start depending on other libraries, and I don’t see a way for customers to reliably avoid doing this. I think it’s ultimately something that we need to handle gracefully.

So yes I want to say this is a bug but I’m unable to credibly do so 😄

jaredpar on Jan 6, 2023

I’m curious to know more about the OmniSharp and Rider scenarios. It makes it sound like they are not using the same analyzer host as the VS IDE or command line build…

If CsWin32 does require a newer version of a “compiler assembly” such as System.Memory, this might be only really resolvable by asking Roslyn to start shipping a newer version of that dependency. It seems like we generally won’t be able to honor the analyzer’s intent for their in-package version of such an assembly to be loaded.

Agree that we should be looking at some way to give a diagnostic which leads users to the root of the problem, not to the component which happened to step on the land mine, so to speak.

RikkiGibson on Jan 6, 2023

It feels like it can be someone’s job to decide how to not ship compiler assemblies in an analyzer package. 🙂

The set of compiler assemblies isn’t static though. What ends up mattering is what is present at runtime, not what the compiler depends on at compile time. What is present at runtime is different between .NET Framework and .NET Core and changes release to release. The pack step can’t actually know what the right answer is.

jaredpar on Jan 5, 2023

I would agree that the solution is probably to share the set of “compiler assembly simple names” on the Framework side, and when such assemblies are found in /analyzer to essentially ignore them and load the compiler’s version instead.

The part I don’t like about our .NET Core solution is the hard coding of names. Yes we have checks to make sure it’s consistent but it still feels off a bit for me in ways that are hard to quantify.

What I was thinking about for .NET Framework was along these lines. https://github.com/jaredpar/roslyn/commit/9d716e128e035873d15953842e0a9b823f5c7ef8

My first version had none of the checks for name or file location. I wanted it to be essentially if the host owns this name then always load from the host. That leads to lots of runtime exceptions though because we end up probing for every single assembly and that felt off. The check and then load feels a bit better.

I am also wondering, though, if it is essentially a bug that CsWin32 is shipping “compiler assemblies” when the compiler host is always going to provide the version that needs to actually be used in order for things to work.

Strictly speaking yes I consider it a bug but I’m also sympathetic to the push back of “okay, so tell me how to not ship the assembly”. The problem I see generators like CSWin32 facing is they have some dependencies and those bring in dependencies transitively that are shared with the compiler. So to be compliant with the “don’t ship compiler assemblies” idea these generators have to know what assemblies we ship, across all versions of the compiler, and prune them out. That’s not realistic I think.

jaredpar on Jan 5, 2023

Never mind I can repro the problem now. The problem was I failed to save before trying to build 🤦

jaredpar on Jan 5, 2023