Permit non-void return types for partial methods (VS 16.8, .NET 5)

[AaronRobinsonMSFT]

The Source Generator feature is being investigated to create a runtime agnostic and AOT compatible P/Invoke stub generator. One of the hard requirements of Source Generators is user written code is read only and cannot be altered by any Source Generator implementation. This requirement makes it difficult to declare methods and use them, but have a Source Generator provide an implementation in the future (i.e. build time).

Possible work-arounds exist by declaring a partial class and having a convention.

// User defined
[FutureMethod("Foo", typeof(int))]
partial class GenClasses
{
}

// Generated in another TU
partial class GenClasses
{
   public static int Foo() { ... }
}

The above could be made to work, but there are issues.

• How does documentation for Foo work?
• What is the IDE scenario here for IntelliSense?
• Users would probably see many red squiggles that make development annoying.

An alternative approach would be a type of forward declaration using partial methods. The below would relax the requirement to have a void return type and thus the generation of the corresponding method could occur. IntelliSense would make sense as would a location for XML documentation in the user defined code.

Note partial methods are also required to be private. The below example does adhere to that requirement, but relaxing that would also be beneficial although not strictly needed.

// User defined
partial class GenClasses
{
   static partial int FooImpl();
   public static Foo() { return FooImpl(); }
}

// Generated in another TU
partial class GenClasses
{
   static partial int FooImpl() { ... }
}

/cc @jaredpar @davidwrighton @jkotas @stephentoub

[YairHalberstadt]

I presume that unlike void private methods, an implementation of the partial method would be required?

[AaronRobinsonMSFT]

@YairHalberstadt I personally have no objections to that, but it would be up to the language teams to make the final decision. I would imagine some complexity if that wasn't the case though.

[agocke]

I independently thought of the same thing when @chsienki and I were brainstorming a while ago. Seems useful. I assumed that non-void return types would require an implementation method.

[jaredpar]

I presume that unlike void private methods, an implementation of the partial method would be required?

Yes indeed. It would also mean that partial methods which have non-void return methods cannot be erased at the call site; there will always be an invocation of the method.

partial methods are also required to be private. The below example does adhere to that requirement, but relaxing that would also be beneficial although not strictly needed.

I think we'd also want to lift that restriction. Think the best way to approach that part is to say that partial methods which have a non-private accessibility are required to have an implementation. That gives it similar rules as non-void return types.

[tannergooding]

If private void partial-methods are not required, and non-void partial methods (any accessibility) are required, would non-private void partial methods be required?

Would it be worth something allowing the user to specify that the void partial method (any accessibility) is required to help catch issues where they expect it to be provided by a source generator and want to error if it isn't?

[tannergooding]

(To clarify, I mean a specific instance is required so we don't break back-compat).

[333fred]

non-private void should almost certainly be required. They're part of a public api, and having them silently do nothing would break abstraction boundaries.

[jaredpar]

@tannergooding

If private void partial-methods are not required, and non-void partial methods (any accessibility) are required, would non-private void partial methods be required?

Yes. As my comment said: partial methods which have a non-private accessibility are required to have an implementation. I deliberately left out return type in that statement.

Would it be worth something allowing the user to specify that the void partial method (any accessibility) is required to help catch issues where they expect it to be provided by a source generator and want to error if it isn't?

Possibly but I'd wait to see a compelling case before adding a new feature to support this. So far we've been discussing relaxing existing restrictions to support source generators. This would be adding a new restriction and is essentially a new feature (need syntax or attribute to support).

[alrz]

Previous discussion: #753

[AaronRobinsonMSFT]

Previous discussion: #753

@alrz That is definitely being discussed here. Should we close that issue in lieu of this one since this is a super-set?

[alrz]

Should we close that issue in lieu of this one since this is a super-set?

Sure, just wanted to add the link just in case. Closed.

[CyrusNajmabadi]

I don't actually understand the need here for this. Addressing hte OP:

How does documentation for Foo work?

The generated Foo includes its documentation. That documentation flows along the same way as nodemal code.

What is the IDE scenario here for IntelliSense?

IDE works because it sees the generated code.

Users would probably see many red squiggles that make development annoying.

That should not happen. The source generator will run and will have generated the method the user can call.

a

[AaronRobinsonMSFT]

How does documentation for Foo work?

The generated Foo includes its documentation. That documentation flows along the same way as nodemal code.

The auto-generated code isn't what the user wrote, so how would it know the documentation for the API? In the below code, where does the XML documentation for the function Foo reside? Any existing documentation tool would be confused by documenting the attribute with the typical XML documentation for a method.

// User defined
[FutureMethod("Foo", typeof(int))]
partial class GenClasses
{
}

What is the IDE scenario here for IntelliSense?

IDE works because it sees the generated code.

That is only true if source generators are continually running in the background. It is entirely possible for this to work similar to XAML, but there are terrible cases there especially during a copy/paste of large swathes of XAML where the editor needs to wait for the language service to keep up. There would be squiggles and general confusion since Foo's signature is being generated and defined by the user in a dynamic manner. As opposed to XAML where there is an inheritance contract to conform to and possibly optimize for the limited set of API signatures that would exist.

In the proposed case, there is a function declaration a head of time which satisfies the location for XML documentation and completely addresses the IntelliSense issue without putting undo pressure on the IDE to try and generate potentially large amounts of code during the edit cycle.

[stephentoub]

The generated Foo includes its documentation. That documentation flows along the same way as nodemal code.

Flows from where?

Let's say this is a regex generator. I want to be able to write this:

/// <summary>Creates a regex that finds hello followed by world, with anything in the middle</summary>
[RegexGenerator("hello.*world", RegexOptions.IgnoreCase)]
internal static partial Regex HelloWorldRegex();

With that I don't have to have a string-based name. I've written the method I'll be calling like any other, except the generator is filling in the implementation, rather than the method magically appearing out of thing air. I've been able to put documentation on the method like I would on any other method. The regular expression that's tied to this method is right there in the method's attribute, not somewhere else in the assembly attributing some less specific thing. Etc.

How does this look without the non-void returning support?

[CyrusNajmabadi]

That is only true if source generators are continually running in the background.

Yes. That's the plan here.

[CyrusNajmabadi]

et's say this is a regex generator. I want to be able to write this:

I would not write it like that. i would just have:

[RegexGenerator("hello.*world", RegexOptions.IgnoreCase, "HelloWorldRegex")]
partial class Whatever
{
}

SourceGenerator would take that and make another part for Whatever that has the HelloWorldRegex property with teh right code injected for it. All features looking at Whatever, will see that member, because that member exists.

[stephentoub]

I would not write it like that. i would just have:

Yeah, to me personally that's much worse, for all the reasons I outlined. And, to the documentation point, it lacks it.

[CyrusNajmabadi]

The auto-generated code isn't what the user wrote, so how would it know the documentation for the API? In the below code,

I'm not really understanding what you mean. When the generator generates the new code it can put whatever comments on it using whatever scheme you want to use to convey information to your generator. It could be embedded in attributes, or comments, or something else.

[CyrusNajmabadi]

With that I don't have to have a string-based name. I've written the method I'll be calling like any other, except the generator is filling in the implementation

This sounds like the original/replace approach that we decided to cancel. these scenarios have a suitable approach with the 'generate a new file' system. It may not be really glamorous, but i don't expect or want it to be. I'd rather have a simple system that can do this, instead of more and more complex features (like actually changing the language) in order to make this happen.

[CyrusNajmabadi]

With that I don't have to have a string-based name.

you don't need a string based name, the following works fine:

[RegexGenerator("hello.*world", RegexOptions.IgnoreCase, nameof(HelloWorldRegex)]

there's a duality here. teh generator can see the above and use it to make the method called HelloWorldRegex. The attribute can also then see that new method and then compile.

[CyrusNajmabadi]

And, to the documentation point, it lacks it.

// This is the documentation to generate on the member.  The source generator can look for this
// and move it over.  The source generator can totally make an informal mechanism to allow its domain
// to pass it the information it needs
[RegexGenerator("hello.*world", RegexOptions.IgnoreCase, nameof(HelloWorldRegex)]

[stephentoub]

The source generator can totally make an informal mechanism

Why would I want that when we already have a long-standing, compiler-supported mechanism for this? I don't want to invent my own mechanisms, worry that an arbitrary comment in just the wrong place is going to be misconstrued by the ad-hoc conventions of the generator.

[CyrusNajmabadi]

There would be squiggles and general confusion since Foo's signature is being generated and defined by the user in a dynamic manner.

That would be weird. We're not going to run diagnostics on hte code prior to source generation. Expectation is we run it after source generation. IN other words, it's part of our design that the code prior to gen is normally expected to be in error because it will depend on the generated code.

In other words, the thing you're trying to fix/avoid here is precisely the use model we're trying heavily to move toward. the expectation is absolutely to not have the definitions or aything else informing the original source code. but all IDE features and whatnot only operate on the post-transformed code.

[CyrusNajmabadi]

Why would I want that when we already have a long-standing, compiler-supported mechanism for this?

Because it would work? And it requires no language changes at all.

I don't want to invent my own mechanisms,

But you're asking us to invent a mechanism.

Right now the benefit of the invention we have is that it is entirely divorced from the language. It is simply: you provide information however you want, and you generate however you want.

We only assume that the generated state of the world is valid and worth doing anything with. We don't need any sort of language handshakes to invent. It's all just an api. Just like analyzers don't require anything special and can rev entirely separately from the lang, we'd like the same for generators.

[AaronRobinsonMSFT]

In other words, the thing you're trying to fix/avoid here is precisely the use model we're trying heavily to move toward. the expectation is absolutely to not have the definitions or aything else informing the original source code. but all IDE features and whatnot only operate on the post-transformed code.

Then how is the generated code ever used? A key invariant of the source generators is that they can't modify any user defined code. How is this new code called/consumed without use of reflection?

[stephentoub]

But you're asking us to invent a mechanism.

I'm asking the language to add a small feature that makes the source generator way more usable and user-friendly, in my opinion.

these scenarios have a suitable approach with the 'generate a new file' system. It may not be really glamorous, but i don't expect or want it to be. I'd rather have a simple system that can do this, instead of more and more complex features (like actually changing the language) in order to make this happen.

And the feedback I'm sharing, and that others here are sharing, and offline conversations from even more folks sharing this, is that this is very suboptimal.

[AaronRobinsonMSFT]

We don't need any sort of language handshakes to invent. It's all just an api.

This isn't a requirement for sure. This simply makes the Source Generator feature usable in more scenarios (e.g. AOT).

[CyrusNajmabadi]

ow is this new code called/consumed without use of reflection?

// this is normal user source:
void Foo()
{
     var v = Whatever.HellowWorldRegex;
}

[Regex(...)] // see above for example
partial class Whatever
{
}

This just compiles. It works because the generator sees whatever it needs and generates:

partial class Whatever
{
    public Regex HelloWorldRegex => ... 
}

It is very intentionally by design that the original source on its own with any compiler would not compile. And it can only compile and have meaning with the generator doing its thing.

[CyrusNajmabadi]

I'm asking the language to add a small feature that makes the source generator way more usable and user-friendly, in my opinion.

Can you explain how it's more user friendly? For users consuming the generator it's going to be the same right? They'll simply see the final state of things and all things like intellisense, docs and whatnot will be the same.

In terms of people creating the generator, i'm happy to work on all sorts of IDE features to make that a pleasant experience. but i'm not seeing the need for a language change.

[CyrusNajmabadi]

Keeping all attributes around makes an existing problem a bit worse, it is not introducing a new problem.

Fair enough. Seemed oogy to have an entirely source-oriented solution make it's way into metadata. But if it doesn't offend people, it's not going to bother me that much :)

[YairHalberstadt]

i.e. all the arguments made apply equally to properties as they do to methods.

To reiterate my point on dotnet/roslyn#40162 (comment), one of the commonly requested use cases for source generators is generating calls to NotifyPropertyChanged from an auto property, without having to create a full property, via usage of an attribute.

This is tricky with the current proposal, but partial properties would make this trivial to do.

[YairHalberstadt]

we keep all attributes, but we remove those deriving from some well known type (like SourceGeneratorAttribute).

You might not even need it to be a well known type. Conditional attribute should work:

using System;
using System.Diagnostics;

[Conditional("sgadfbvsedbbs")]
public class SourceGeneratorAttribute : Attribute {

}

public class MyAttribute : SourceGeneratorAttribute 
{
     [MyAttribute]
     public void M() {}
}

The usage of MyAttribute is not emitted into metadata: https://sharplab.io/#v2:EYLgtghgzgLgpgJwDQxASwDYB8ACAmARgFgAoHABgAIcCA6AETQgHMA7Ae1jQGMoBuUqQDaAYXasAJmhhpxEDAAoARFGYQJAM2AA3KHAnBgUJQEoAuqRwBmankoBldgFcE3OAHE4rRBBjsEAIIwMAhowE7wlCCUQSFhEXCUAN6CJAC+qda2lACyAJ6xoeGR0Y4ubp7eCL7+hfGRpCkklC0tQvl1xXAWza3UNjgALLkKJskZ6UA==

(With thanks to @HaloFour)

[alrz]

Thinking out loud here, this could be actually used where you need replace/original, using some lightweight code convention that is completely in the control of the aurhor, take cached method example:

[Cached]
public partial int Compute(int i);
private int ComputeImpl(int i) {}

The generator will look for an Impl (or whatever) method and use it in the cached implementation.

That said, I support
partial properties idea, it would enable other (all?) scenarios previously discussed in the context of replace/original feature.

[RikkiGibson]

Right now the 'partial' modifier is required to be the last modifier before the type. As part of this feature should we relax this requirement?

[HaloFour]

@alrz

That's an interesting use case. I could see the (ab)use of partial properties as stubs for a generator to fill in a specific aspect:

[IPNC]
public partial class Foo {
    public partial string Name { get; set; }
}

// generated:
public partial class Foo : INotifyPropertyChanged {
    public event PropertyChangedEventHandler PropertyChanged; 

    private string _name;
    public partial string Name {
        get => _name;
        set {
            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Name)));
        }
    }
}

The obvious problem with it is that it's not composable, you'd only ever be able to apply a single aspect.

I honestly hope that the team will reconsider replace/original or some other approach for AOP in the future. IMO that's where the majority of compelling use cases are, and I think people are going to be quite frustrated once they realize the limitations of what will be shipped. I personally feel that a compiler-provided approach via source generators would be significantly easier to use, reason about and debug than the existing AOP solutions.

[alrz]

@RikkiGibson

Right now the 'partial' modifier is required to be the last modifier before the type. As part of this feature should we relax this requirement?

I've done a standalone implementation of that long time ago and it's already being considered as a pre-requirement for records (dotnet/roslyn#42097)

[alrz]

@HaloFour

The obvious problem with it is that it's not composable, you'd only ever be able to apply a single aspect.

Since the final method is being generated at last, you can build the composability into your generator with some code tricks, no?

Also, replace/original would not solve that because all generators see the original compilation. That's a non-issue for partial because there can only be a single impl method.

IIRC using multiple replace impls remained an open issue due to the ordering question.

[HaloFour]

@alrz

Since the final method is being generated at last, you can build the composability into your generator with some code tricks, no?

I could see that if you were composing multiple aspects supported by the same generator, but only if the generator were crafted to interpret those multiple aspects and emit a single implementation taking all of those aspects into consideration. Other than that I don't see composability being practical.

IIRC using multiple replace impls remained an open issue due to the ordering question.

Isn't it still an open question? What happens if you reference multiple generators now and two of them want to emit the same types or members? Can each generator see what the other generator has already emitted? Do they run in a deterministic order and at deterministic times?

[alrz]

Isn't it still an open question?

I don't think so. The issue seems to be resolved because we don't depend on it anymore.

What happens if you reference multiple generators now and two of them want to emit the same types or members?

I wouldn't expect anything other than a compile-time error.

Can each generator see what the other generator has already emitted? Do they run in a deterministic order and at deterministic times?

No, and the order is irrelevant as each generator can only see the original compilation from source.

[HaloFour]

@alrz

No, and the order is irrelevant as each generator can only see the original compilation from source.

Sounds like that would make it practically impossible to compose multiple aspects backed by different generators.

[alrz]

@HaloFour

Sounds like that would make it practically impossible to compose multiple aspects backed by different generators.

One possible solution is to have a plugin-based framework on top of generators API to be able to chain multiple generator "plugins" with a defined execution order. Though I think that would be only widely usable if it's from MS.

[HaloFour]

@alrz

Execution order and convention for emitting multiple partial methods into which each generator can put its implementation and chain to the next. Doable, but I don't think it's practical without some major backer. But maybe necessity will drive it.

[stephentoub]

Kindly prioritize this.

I'm not sure why this issue is still open. C# 9 already supports partial methods with non-void return types (you just need to explicitly specify a visibility), e.g.
https://sharplab.io/#v2:EYLgtghglgdgPgAQEwEYCwAoBBmABABwgCcAXKCAG12VwGFcBvTXF6vQ08q2E3AWQAUASgDczVjgLEylXD37DcAXgB8uFGIwBfIA

[Joe4evr]

Is there any update on this?

It's already implemented and working.

in C# API can look like this but wont work

For non-void returing partial methods, you must include an accessibility modifier.

@stephentoub

I'm not sure why this issue is still open.

Feature issues stay open until they're documented in the ECMA spec. See also the tag Implemented Needs ECMA Spec.

[Neme12]

@alrz

Thinking out loud here, this could be actually used where you need replace/original, using some lightweight code convention that is completely in the control of the aurhor, take cached method example:

[Cached]
public partial int Compute(int i);
private int ComputeImpl(int i) {}

The generator will look for an Impl (or whatever) method and use it in the cached implementation.

That said, I support partial properties idea, it would enable other (all?) scenarios previously discussed in the context of replace/original feature.

I feel like that's not a good use case at all. If there is a convention, you could just declare ComputeImpl with the cached attribute on it and the source generator would create the Compute method that delegates to it. That way, you wouldn't have to declare the same method signature twice. This would just make you duplicate the signature without any benefit. The real use case for partial is where you don't already have any existing signature a want to specify it somewhere.