Compile can succeed with generator being absent

[maryamariyan]

Given a certain attribute that is already present in the framework (e.g. LoggerMessageAttribute in Microsoft.Extensions.Logging) but in case the source generator does not run, then the user would have no ability to see a compile-time error indicating that (in the logging case) no method body was generated due to the generator not being present.

Example:

    partial class C
    {
        [LoggerMessage(...)]
        public static partial void M1(ILogger logger, LogLevel level);
    }

Link to dotnet/runtime#51064 (comment)

[CyrusNajmabadi]

This is solvable by having that type come with an analyzer that checks this for you. Analyzers run after source generators, so it can validate that the SG ran.

[jinujoseph]

cc @chsienki @jasonmalinowski what would be best practice here ?

[CyrusNajmabadi]

I would think the above would have to be the solution. The above is legal code. It has to continue to compile as per hte rules of the language. If teh owner of that attribute wants it to not compile if the requisite SG code is not there, then an analyzer should be provided with that API that ensures that that's the case. :)

[jasonmalinowski]

Also @maryamariyan why did the generator not run? Is this a situation where the user can configure their projects in a way where that would be a problem, or just that some bug happened during development?

[ericstj]

I think we're more concerned about a case where the user was intentionally trying to use the code-generator but ran it in an environment that couldn't ensure the generator was passed to the compiler. For example: runtime compilation, build system that doesn't run all SDK targets, etc.

In such a case the user couldn't ensure an analyzer was passed to the compiler any more than they could the generator, so adding redundancy in an analyzer doesn't really help anything.

This is the first time we're adding a new asset type to the .NETCore framework that is required for completeness. Imagine systems out there that were replicating framework design time and didn't need to know about generators. I'd really like those systems to fail if someone gave them code that depended on code-generators without those generators being present.

[sharwell]

In such a case the user couldn't ensure an analyzer was passed to the compiler any more than they could the generator, so adding redundancy in an analyzer doesn't really help anything.

This is exactly the issue I was concerned about when I read this.

[terrajobst]

@ericstj

This is the first time we're adding a new asset type to the .NETCore framework that is required for completeness. Imagine systems out there that were replicating framework design time and didn't need to know about generators. I'd really like those systems to fail if someone gave them code that depended on code-generators without those generators being present.

My suggestion is: because this is the first time we're doing this, let's wait and see whether that's a concern that actually appears in the wild and root cause what happened.

My gut is that the problem won't be the that the generator wasn't passed to the compilation, but more that someone else, for example, defined the attribute local that the generator didn't recognized or made other mistakes in the authoring that prevented the generator from running or producing code.

[ericstj]

My suggestion is: because this is the first time we're doing this, let's wait and see whether that's a concern that actually appears in the wild and root cause what happened.

We can't go backwards from what we're doing now. Once the attribute is in the framework it's there forever. I was hoping we'd have a mechanism that was encapsulated within the generator for interacting with the generator. That way it either works, or it doesn't. There's no half-way.

defined the attribute local that the generator didn't recognized or made other mistakes in the authoring that prevented the generator from running or producing code.

Those would just be either bugs in the generator, or errors raised by the generator that tell the developer they've done something wrong. We need to hold generators to the same bar as the compiler or libraries. There should never be silent failure. There shouldn't be syntax that "sometimes works"

[CyrusNajmabadi]

There shouldn't be syntax that "sometimes works"

This is how partial had always worked though. If you fall to run whatever tool you have that generates the other part of the partial... Then you will just see that partial call get elided. It's not clear what is different now.

Can an analyzer just ship alongside this api?

[ericstj]

The difference here is the user is expressing that a generator fill in the partial method by applying that attribute. The problem is that the partial method will not be filled if the generator isn’t provided. An analyzer isn’t a solution because the analyzer has the same problem as the generator (they are specified in the same way). The analyzer cannot be provided by the same component which provides the API.

A possible solution would be for the generator to define the attribute. This doesn’t work well today because the generator persists the attribute definition in the user’s assembly. If a generator could define a compile time only attribute, suggested by @stephentoub, that would work.

Another possible solution, suggested by @layomia, is the attribute usage somehow “demands” the generator. Forcing a failure/warning if the generator was absent.

Another possible solution is for the generator to somehow add a unique reference that it carries within itself (eg: embedded resource + intellisense) which defines the attribute and ensures its reference is not persisted (Conditional).

[CyrusNajmabadi]

The analyzer cannot be provided by the same component which provides the API.

Can you expand on this piece? Why would that be? It would make a ton of sense to me that a component provide its own analyzers here. Thanks!

The difference here is the user is expressing that a generator fill in the partial method by applying that attribute. The problem is that the partial method will not be filled if the generator isn’t provided.

Sure... but that's true of any solution that uses partial methods. The idea was always that one part is provided optionally. It's always been the case that if you didn't provide the piece that provides that optional part then you just get the code elided.

[ericstj]

Can you expand on this piece? Why would that be? It would make a ton of sense to me that a component provide its own analyzers here.

Analyzers and references are separate parameters. You cannot guarantee both are present, even if they are the same assembly. A reference cannot say it “requires” an analyzer. If it could that would solve this (we’d require the generator though rather than an analyzer).

but that's true of any solution that uses partial methods

We aren’t claiming there is a problem with partial methods, they just happen to illustrate this limitation with source generators.

[stephentoub]

For proponents of an analyzer solution, what does that look like for languages that don't support Roslyn analyzers? We ship an attribute in the core libraries consumable by any language that targets those libraries, but the attribute can only meaningfully do its job when consumed by C#. We can also ship an analyzer that would flag incorrect use by C# and VB. What's the answer for F# or C++/CLI or any other .NET language?

[CyrusNajmabadi]

This is not an ide concern. Moving to compiler. If they want to tell people somehow that a generator did not run, that would be a suitable place to do it.

[terrajobst]

@stephentoub

Do you have something in mind? We could mark the attribute somehow as being C#/VB/F# specific and ask compilers to honor it.

[stephentoub]

@stephentoub

Do you have something in mind? We could mark the attribute somehow as being C#/VB/F# specific and ask compilers to honor it.

That would partially address things, a way to flag an attribute as being language-specific. However, it's not complete, as even in C# it's possible for the attribute to be employed but the analyzers not run. For example, if I have:

[GeneratedRegex(...)]
static partial void Oops();

and the analyzers don't run, the erroneous use of this attribute won't be flagged.

I'm not sure there's a great answer that fully addresses all possible issues. But I'm also not sure how much of a problem this actually is in practice; we've arranged things to make it harder to have these problems, e.g. using partials that fail to compile if the source generator doesn't supply an implementation, having the analyzers come in the same package as the attributes, etc. Have we heard of lots of folks running into this?

[terrajobst]

I haven't. It's partially because of these things:

• .NET Core SDK support story is basically tip. IOW, even if you target older runtimes, folks still tend to use latest SDK
• .NET Framework is mostly latest VS, which brings latest tools and we don't invest much in .NET Framework based source generators
• Most people don't heavily mess with the build environment to disable / inject source generators themselves

At the same time, we already have CompilerFeatureRequiredAttribute. It might be worth considering how we would expand that.

[jaredpar]

Have we heard of lots of folks running into this?

From the compiler side I've maybe seen one or two instances of this. At least one was customers specifically disabling analyzers in the build. There are definitely customer issues around generators not executing properly but they're in the variety of the compiler issued errors running the generator, not generator didn't run (most typical example is loading a new generator in an old compiler leads to version mismatch diagnostic).

At this point if we've not seen any significant customer feedback on this I think we should move this to the backlog. Can revisit if customer feedback reveal it's a problem.

[terrajobst]

Works for me.