Roslyn: [Proposal] add supersede modifier to enable more tool generated code scenarios.
Some C# language features exist primarily for tooling scenarios. Designers, build time tools and other code generators produce source code that compiles together with and augments user written code. Partial classes and partial method are two of these.
With partial classes, a tool can generate source files that make additional declarations for a class, adding members, interfaces and attributes on top of what the user wrote.
Partial methods allow tools to declare signatures for methods that don’t actually exist, but that can be supplied elsewhere by the user. In this way the tool is free to generate code invoking this partial method without requiring the method to have been fully supplied. They are limited to private void returning methods so that they can vanish during compilation if the user has not supplied a full declaration with a body for the partial method.
In this way, partial methods are a clever trick enabling tool generated code to interact with user written code in more than just the simple additive way that partial classes provide. However, it is a very narrow trick. It allows the user to enhance the behavior of the tool generated code at well-defined spots by providing method implementations, but it does not allow the tool generated code to enhance the behavior of the user written code.
For example, imagine I wanted a tool to generate code that adds entry and exit logging to my user written code (without actually changing my source files or post processing the binary.) I could pervert the use of partial methods to approximate this, by inverting how they are normally used. I could pre-establish an agreement between my code and the tool (probably established by the tool), and liberally sprinkle invocations to logging methods throughout my code base, and then let the tool generate the bodies.
// user written code
public partial class MyClass
{
private partial void log_entry(string name);
private partial void log_exit(string name);
public void Method1()
{
log_entry(“Method1”);
// something totally awesome here
log_exit(“Method1”);
}
}
// tool generated code
public partial class MyClass
{
private partial void log_entry(string name)
{
Console.WriteLine(“entered {0}”, name);
}
private partial void log_exit(string name);
{
Console.WriteLIne(“exited {0}”, name);
}
}
Then I could choose to run the tool or not, so that the calls to log_entry and log_exit either had an implementation at runtime or not.
But I would have to manually place calls to these methods everywhere throughout my code. And I’d have to do this over and over again for each additional tool that needed to interact with my code. A very tedious prospect, and one better suited for tools generating code than my tired fingers.
A better solution would be one that allowed the generated code to enhance the user written code directly, by replacing it or overriding it, and placing all the burden of declaration into the tool generated code.
Proposal: Superseding members
The supersedes feature enables just this. It allows class members to override members defined in the same class, by superseding their declaration with a new declaration using the supersede declaration modifier.
// user written code
public partial class MyClass
{
public void Method1()
{
// something totally awesome here
}
}
// tool generated code
public partial class MyClass
{
public supersede void Method1()
{
Console.WriteLine(“entered Method1”);
superseded();
Consolel.WriteLine(“exited Method1”);
}
}
In this scenario the tool generated Method1 supersedes the user written Method1. All invocations are directed to the tool generated method. The tool generated implementation can then invoke the user written method using the superseded keyword that points at the member that was superseded by this declaration.
Using this technique, I could have a tool generate my boilerplate INotifyPropertyChanged implementations.
// user written code
public partial class MyClass
{
public int Property { get; set; }
}
// tool generated code
public partial class MyClass : INotifyPropertyChanged
{
public event PropertyChangedEventHandler PropertyChanged;
public supersede int Property
{
get { return superseded; }
set
{
if (superseded != value)
{
superseded = value;
var ev = this.PropertyChanged;
if (ev != null) ev(this, new PropertyChangedEventArgs(“Property”));
}
}
}
}
Multiple Generators
Of course, since the primary scenario for this feature is tool generated code and there is likely going to be times where there is more than one tool wanting to supersede a user written member, some mechanism should exist that allows one bit of tool generated code to supersede another bit of tool generated code without requiring them to know about each other.
Proposal: superseding superseded members
We can allow more than one superseded member declaration to exist at the same time. One of the superseding members directly supersedes the original member. Another superseding member then supersedes the prior superseding member, and so on. Thus a chain of superseding members exists similar to how multiple overrides might exist on the same member in an inheritance hierarchy. In fact, it is somewhat helpful to think of superseding as directly comparable to overriding. If you imagine a series of overrides of a member in a vertical inheritance hierarchy, you could image superseding as kind of a horizontal overriding (between partial class declarations).
// user written code
public partial class MyClass
{
public void Method1()
{
// something totally awesome here
}
}
// tool #1 generated code
public partial class MyClass
{
public supersede void Method1()
{
// do something interesting
superseded();
}
}
// tool #2 generated code
public partial class MyClass
{
public supersede void Method1()
{
// do something else interesting
superseded();
}
}
Order of supersession
The only problem we have is determine which superseding member is the first, the next and so on. The order is going to matter, because it determines which gets called first and that can affect behavior of the entire composition.
Unlike member overrides that occur in derived classes that specifically call out the names of the classes they are deriving from, the supersede syntax doesn’t directly identify the precise member that is being superseded. And that’s a good thing, because if it did, then code generating tools would absolutely need to know about each other, so they could reference each other’s declarations.
Instead, we need a different technique (other than naming) to identify ordering between the superseded members.
Proposal: lexical order
We can use lexical order to determine the order in which members are superseded. (For multiple source files, consider the files concatenated in the order supplied to the compiler.)
- The member without the supersede modifier is the base implementation.
- The superseding member that is earliest in lexical order is the first to supersede the base.
- The superseding member that is next in lexical order supersedes the previous superseding member, and so on.
In this way, no additional syntax needs to be invented to support having multiple superseding members for a single base member. The natural order of their declaration determine the order for superseding.
(This does not consider how the generated source files are given their order.)
mattwar
All 73 comments
Great proposal!
Are there any other options for changing the way the original member is invoked? I like the new "superseded" concept and I think it should feel similar to the way that "mybase" operates. Perhaps it should be called like extended.Member() ? This would also allow other members to potentially differentiate if they want the new declaration or, perhaps, want the original.
// user written code
public partial class MyClass
{
public int Property { get; set; }
}
// tool generated code
public partial class MyClass : INotifyPropertyChanged
{
public event PropertyChangedEventHandler PropertyChanged;
public extended int Property
{
get { return extended.Property; }
set
{
if (extended.Property != value)
{
extended.Property = value;
var ev = this.PropertyChanged;
if (ev != null) ev(this, new PropertyChangedEventArgs(“Property”));
}
}
}
}
TonyValenti
on 17 Sep 2015
@mattwar, besides allowing multiple supreseeded implementations of the method, what else can you do with this that you can't with the current implementation?
paulomorgado
on 17 Sep 2015
This is interesting in the context of enabling AOP without post-compilation rewriting (a la PostSharp). However, I don't actually see any part of the proposal that specifies how you would define an aspect or apply it to code. You definitely would not, for example, want to be manually invoking some tool in Visual Studio every time you want a class to implement INotifyPropertyChanged.
Besides being a poor workflow, it would leave your source with no indication what magical properties are being added to it elsewhere. I think you absolutely need to have either new keyword(s) to express these behaviors, or use Attributes.
Lastly, the proposal for how you order superceeded items seems like a total non-starter to me. Having the build order determine semantic meaning of code is dangerous and completely non-obvious to anyone reading the code. The ordering would need to be part of the aforementioned aspect keyword/attributes. This is what PostSharp does.
MgSam
on 17 Sep 2015
Not what I envisioned for AOP at all. I expected something more like an analyzer/code-fix where the processor would be handed the syntax tree, transform it and then return it to the compiler during the compilation process (although being able to run it separately and view the generated source representing the syntax tree would be very useful for debugging scenarios.) Would a processor in this case be expected to parse source or trees or either?
I am also curious as to when whatever generates this source would be used. Having worked with T4 it's not the most pleasant experience in the world (particularly paired with an SCM plugin, and I'm looking at you Entity Framework). Even if source were to be spit out of such a processor I'd want it easily integrated into the compilation process.
I do think that there are scenarios where order of "supercession" would be important. A processor adding validation logic may want to bail ahead of a processor adding transaction logic or similar.
HaloFour
on 17 Sep 2015
@TonyValenti I considered having superseded refer to the instance (like base does), but it turns out the scenario is quite different. With base, the whole class is overridden, so its possible to use it to reference any member in the base class. Yet with supersede, each member is superseded individually. The class itself is never superseded. All parts of the partials are the same class. So it only made sense to have it reference the other member directly.
mattwar
on 17 Sep 2015
@paulomorgado what are you referring to as the current implementation? Do you mean the partial methods I used as an example, or just any/all existing language features?
mattwar
on 17 Sep 2015
@MgSam, @HaloFour This proposal is separate from any discussion of actual implementation of code generating tools or how they would interact with the compiler. This feature is meant strictly as a way to enable such generators should they exist with a means to add behavior to code already written without resorting to IL rewrites, etc.
mattwar
on 17 Sep 2015
@HaloFour it would be important for the code generators to be able to order their generated declarations.
mattwar
on 17 Sep 2015
@mattwar That makes sense, but I don't think its possible to get a complete picture on the merits of this proposal without a complimentary proposal for how you would create and manage aspects. Very similar to how primary constructors/records/tuples/pattern matching need to all be considered together and how they'll interact rather than trying to only build each piece in isolation.
MgSam
on 17 Sep 2015
@MgSam I agree, it would be better to consider it with a full code generation proposal. Maybe I should write one.
mattwar
on 17 Sep 2015
I'm not opposed to the keyword pairs supercede / superseded but are others being evaluated?
Perhaps aspect / inner or outer / inner?
As to ordering, assuming the generation tools could provide a sequence to filenames lexical order seems good enough. The built in AOP tools could process a file like this:
- MyClass.cs
- MyClass.g.01.cs
- MyClass.g.02.cs
Community tools for other processes (some custom t4 processor?) could create things like MyClass.timings.cs and since "t" comes after "g" it would come after it in order in terms of processing the partial class declaration. The built in tool may derive order from some predefined mechanisms but it will presumably generate these aspect classes in a particular order and can then decide to name them so that they sort in a defined manner.
Should there also be some strict compilation rules around matching signatures?
Should this compile:
public partial class MyClass
{
public void Method1(int a, int b)
{
// something totally awesome here
}
}
public partial class MyClass
{
public supersede void Method1(int b, int a)
{
superseded(b, a);
}
}
public partial class MyClass
{
public supersede void Method1(int a, int c)
{
superseded(c, a);
}
}
bbarry
on 18 Sep 2015
@bbarry
Perhaps intercept and intercepted?
The file name issue would fall under the lexical ordering as stated above. To my knowledge the C# compiler doesn't currently care what order those files are provided, and the file name itself is irrelevant. Going by the order in which the files are passed to the compiler could at least be deterministic. Basing the order by file name, particularly using a natural sort, could cause issues in different locales/languages.
I would imagine that the same rules would apply to supercession as applies to partial methods in that the signature must match exactly.
HaloFour
on 18 Sep 2015
Can we rather use typesafe macro expansions? The proposed syntax looks rather...unwieldy.
tpetrina
on 18 Sep 2015
@mattwar,
current == C#6
Other than allowing havoc with multiple generators running over each other on a not yet defined or possibly never deterministic way, what can you do given this proposal is implemented that you can't now (C#6)?
paulomorgado
on 18 Sep 2015
@paulomorgado It is not what you can do with it that you could not do before, it is what tool generated code can do for you that it could not do before without post build rewriting. For example, a tool can generate all the boilderplate code you need to implement INotifyPropertyChanged for classes you declare using normal C# syntax. You and the tool get to collaborate on the content of classes and members. The existing partial classes feature only gets you part way there.
mattwar
on 18 Sep 2015
A review of this proposal occurred a while back (in 2015) that included some brainstorming of alternate keywords to use for supersedes and superseded.
Instead of supersede/superseded use replace/original.
// user written code
public partial class MyClass
{
public void Method1()
{
// something totally awesome here
}
}
// tool generated code
public partial class MyClass
{
public replace void Method1()
{
Console.WriteLine(“entered Method1”);
original();
Consolel.WriteLine(“exited Method1”);
}
}
This would get rid of some of the awkwardness of everyone disagreeing on how to spell the keywords.
mattwar
on 21 Jan 2016
The original() is good and ismple, but how we call original other method (for example Methos2) from replaced Method1, if Method2 also was defined in replacement class ? Maybe there should be additional kewyord called original, so we could write:
public replace void Method1()
{
Console.WriteLine(“entered Method1”);
original(); // or original.Methos1();
original.Methos2();
Consolel.WriteLine(“exited Method1”);
}
public replace void Method2()
{
...
}
What about to use replacement to classes from libraries ? This should be possible at least to app code - not altering original methods in libraries, but use wrappers.available only to app code.
vbcodec
on 14 Mar 2016
Would it be possible to alter compiler generated code for records?
sealed class Person(int Id, string Name);
partial class Person {
replace bool Equals(object other) {
return (other as Person)?.Id == this.Id;
}
}
If calling the original is mandatory, I strongly suggest to use decorate instead of replace. And if it's not, perhaps only one of replaced members is allowed to NOT call original?
alrz
on 18 Mar 2016
I'm still :-1: on this proposal.
If the developer of the extended class intended for it to be extended, it should have done it in a well thought and documented way.
paulomorgado
on 18 Mar 2016
If the developer of the extended class intended for it to be extended, it should have done it in a well thought and documented way.
@paulomorgado I don't get it, you mean if the class has partial declarations it should be documented? Sure, but this doesn't go beyond assembly boundary so there is no "extended" classes, just partial classes which we had before.
alrz
on 18 Mar 2016
"extended" in the sense that a partial definition (not a partial class) is added to the generated definition.
Isn't this whole proposal about partial class definitions and a way to override/supersede a default definition?
paulomorgado
on 18 Mar 2016
"extended" in the sense that a partial definition (not a partial class) is added to the generated definition.
@paulomorgado replace is only useful when you have a partial class and it's up to you to use #5561 to add a partial declaration or not. Actually I thought that original is mandatory in a replace declaration and proposed #9178 but it seems that it's not (accourding to #9532).
Isn't this whole proposal about partial class definitions and a way to override/supersede a default definition?
It's actually about decorating existing members so that one can add additional logic to a method/property. I'm not sure that I understand your objection though.
alrz
on 18 Mar 2016
Would really love to see this, it would enable a whole bunch of cool stuff!
aL3891
on 4 Apr 2016
This would definitely enable a lot of cool stuff that currently requires external tools like PostSharp, I would really love to see this come out.
How about introducing a mechanism to protect the members of a partial class from being '_replace_'-ed? I think that would be a useful scenario for library developers where the library is open source distributed for the clients to customize & extend. This mechanism protects and guarantees that certain pieces of the partial class cannot be _replaced/superseded_ in a new file for this partial class...
For example use a new keyword like _nonreplaceable_
_// File-1_
nonreplaceable void NonReplaceableMethod()
{
// some logic goes here...
Method2();
}
void Method2() { }
_// File-2_
replace void Method2()
{ /* custom logic here */ }
replace void NonReplaceableMethod() **// this will generate compiler error**
{ /* custom logic here */ }
rn-3
on 15 Apr 2016
@rn-3 What are the scenarios where your proposal can be useful?
If the library users have the source code of the library, they don't need to supersede, they can just edit the method in question and recompile the code. And if they don't have the source code (and don't bother to decompile the library, or the assembly in question is signed), they cannot add class parts: supersede doesn't (and cannot possibly) work across assemblies.
vladd
on 15 Apr 2016
The library users will have source code but editing the original files would not be recommended since that would break pulling in future updates to the library. The partial classes would be used to customize or extend the library by adding new files and not modifying original source files.
Class parts do not work across assemblies – that’s the reason for distributing the library along with the source files & projects...
From: vladd
Sent: Thursday, April 14, 2016 3:39 PM
To: dotnet/roslyn
Cc: rn-3
Subject: Re: [dotnet/roslyn] [Proposal] add supersede modifier to enable more tool generated code scenarios. (#5292)
@rn-3 What are the scenarios where your proposal can be useful?
If the library users have the source code of the library, they don't need to supersede, they can just edit the method in question and recompile the code. And if they don't have the source code (and don't bother to decompile the library, or the assembly in question is signed), they cannot add class parts: supersede doesn't (and cannot possibly) work across assemblies.
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
rn-3
on 15 Apr 2016
@mattwar:
According to the latest spec (?) of this feature, it seems that extern methods cannot be replaced. Any reason why? It seems that at the moment, extern is only used for internal CLR calls as well as [DllImport] scenarios; with code generation, extern could finally become more broadly applicable for all situations in which you don't care about the original method, but only the method's signature is important for code generation scenarios. It would save you from writing meaningless empty bodies if the original is to be fully replaced anyway.
For instance, to kick off code generation for some method signature T M(int f, out S s), instead of the verbose declaration
public T M(int f, out S s)
{
s = default(S);
return default(T);
}
you could just write the slightly shorter
public extern T M(int f, out S s);
Admittedly, it's a minor benefit, but what's the reason for explicitly not allowing it? In fact, that use case is the very definition of why extern was added to the language in the first place: The body of the method is defined elsewhere - whether it is runtime support as for [DllImport], CLR internal calls, or code generation should be irrelevant, in my opinion. I do agree with not being able to replace abstract methods.
axel-habermaier
on 18 Apr 2016
@mattwar:
Also, another question: Am I allowed to replace getter-only properties? With what? Would the following work? Which Y is assigned in the constructor?
partial class X
{
public X()
{
if (someCondition)
Y = 99;
}
public int Y { get; } = 33;
}
partial class X
{
public replace int Y => someCondition ? 2 : original;
}
@axel-habermaier about extern, that is certainly something that could be covered by this proposal though its not explicitly mentioned. I've had others mention it as well.
As for your example, yes you could replace the getter-only property just as you've written.
mattwar
on 18 Apr 2016
@mattwar What is the reason to require these to match?
- Accessibility -- can't be absent?
- Attributes on the member, parameters, and return type -- can't be merged?
- Set of accessors -- can't be added?
First two just make replacement merely more verbose. And the last one prevents to add a setter to a property in replaced member.
alrz
on 23 Apr 2016
@alrz I don't recall making any detailed claim that would rule any of those out.
mattwar
on 23 Apr 2016
@mattwar: It's in the spec:
The following must match when replacing a member: 1. Signature: name, accessibility, arity, return type, parameter number, parameter types and ref-ness 1. Parameter names and default values (to prevent changing the interpretation of call-sites) 1. Type parameters and constraints 1. Attributes on the member, parameters, and return type (including this for extension methods) 1. Set of accessors in properties and events 1. Explicit implementation of the member 1. Modifiers: sealed, static, virtual, new, and override.
Also, why the following restriction?
The default constructor can be added by a generator but not replaced.
axel-habermaier
on 23 Apr 2016
@axel-habermaier Hm, how would you implement constructor replacing? If you are going to convert the replaced constructor into a plain method, you won't be able assign to readonly fields in it. If you are going to convert the replaced constructor into a chained constructor, your only option is to call it at the _beginning_ (and you'll need to invent a fake signature as well, you can't just rename it -- but this seems to be a lesser problem).
vladd
on 23 Apr 2016
@alrz I would assume the reason is the principle of least surprise. If I see a getter-only protected property, and see some attributes on it, I don't expect it to be in fact a public getter/setter property. From my point of view, the mental model of the replaced properties (as well as methods etc.) should be "adding some minor tweaks/boilerplate on the existing implementation", not more.
vladd
on 23 Apr 2016
@vladd: The spec specifically allows constructor replacement, it only doesn't allow default constructor replacement. Wait, does "default constructor" always refer to the zero-parameter constructor generated by the compiler if there are no other constructors? In that case, it makes sense that it cannot be replaced, as there is nothing to replace in the first place.
axel-habermaier
on 23 Apr 2016
@vladd
how would you implement constructor replacing? If you are going to convert the replaced constructor into a plain method, you won't be able assign to readonly fields in it.
It wouldn't be a separate method, replace is solely a compile-time transform so when you replace a constructor, it literally replaces the constructor. You can't do this with existing types in other assemblies because you need to mark the type as partial.
I would assume the reason is the principle of least surprise
replace essentially causes surprises because you can replace the whole method body with whatever you want without any further argumentation, unless you required an attribute on them which is totally up to you. So adding a setter to a getter-only property wouldn't be much of a surprise, however, I agree that it shouldn't be able to _remove_ accessors, but I don't see any problem if we could _add_ accessors in the replaced property. The thing is that there are chances that you are aware of generators in your project. So you shouldn't be surprised by any of these in the first place.
If I see a getter-only protected property, and see some attributes on it, I don't expect it to be in fact a public getter/setter property.
I didn't say it should be able to _change_ the access modifier, I said it should be able to be _absent_ in the replaced method, so when you have a public/protected or whatever member, you can replace it with a matching member _without_ any access modifier on it. This is what we currently have in partial classes, access modifiers must be the same in all parts or can be just absent. However, it should be possible to _specify_ the access modifier in the replaced member when it's not specified in the source. In other words they should all be agreeing with each other, not conflicting, regardless of the member being a replace or otherwise.
@axel-habermaier
does "default constructor" always refer to the zero-parameter constructor generated by the compiler if there are no other constructors? In that case, it makes sense that it cannot be replaced, as there is nothing to replace in the first place.
If the source type does not have a default constructor you can simply add it in a partial type without replace modifier on it. If the source type does have a default constructor then it has a body and it is _something_ to replace.
partial class C1 {}
partial class C1 { public C1() { ... } }
[A] public partial class C2 : C3 { [A] public C2() : base(...) { ... } }
// absent/additive/matching ctor initializer, class base, access modifiers
[B] partial class C2 : IA { [B] replace C2() { /* probably calls `original` */ } }
I think for constructors we should also be able to either omit the constructor initializer in the replaced member, or it should match with the original and if the source constructor does not have any constructor initializer you should be able to add it in the replaced constructor. Just like _class-base_ in partial types.
As for attributes it's a little more complicated. The question is that you want to replace attributes as well or you just want to merge them all? Note that partial methods do merge attributes (§10.2.7):
- The attributes in the resulting method declaration are the combined attributes of the defining and the implementing partial method declaration in unspecified order. Duplicates are not removed.
- The attributes on the parameters of the resulting method declaration are the combined attributes of the corresponding parameters of the defining and the implementing partial method declaration in unspecified order. Duplicates are not removed.
I think it should be clear that what you want to do in these scenarios because both option might be required depending on the use case. But if I hadn't a choice, I would definitely go with combination.
alrz
on 23 Apr 2016
@axel-habermaier That's Chuck's spec corresponding to what he's implemented for first round so we can get feedback. It's not a final definition of the feature.
mattwar
on 24 Apr 2016
@alrz
It wouldn't be a separate method,
replaceis solely a compile-time transform so when youreplacea constructor, it literally replaces the constructor.
Well, are we discussing the proposition as it's stated at the top of this issue? The current proposition doesn't transform methods at compile time, it just renames the existing methods and adds a new method which has [a possibility] to call the original one. As far I understand, this is why partial is required, because the tools are going to produce new source files rather than modify the compilation process. And as far as I understand, this is how the user would be able to debug the superseded code: _all the instances of method will be available in the source_.
So for the methods there is no magic going to take place: just one plain old method calls another one.
I expect that for the constructors there will be the same technique used (why otherwise?), so the question remains.
I said it should be able to be _absent_ in the replaced method, so when you have a
public/protectedor whatever member, you can replace it with a matching member _without_ any access modifier on it.
Completely agree here.
So adding a setter to a getter-only property wouldn't be much of a surprise
... but this is something I don't agree. If I have in my code a getter-only property, I personally would be very surprised if assignment to the said property compiled. (Moreover, adding a setter to a property changes the meaning of assignment in constructor!) And by the way, adding a dummy setter is not too much of a burden for the developers. My point is that one shouldn't be required to look through all the generated files in order to find out whether the property has a setter.
vladd
on 24 Apr 2016
@vladd
Moreover, adding a setter to a property changes the meaning of assignment in constructor
Replacing a read-only auto-property with anything other than a read-only auto-property changes the meaning of assignment in the constructor, i.e. a compile-time error.
// perfectly fine
partial class C { int P { get; } C() { P = 1; } }
// results in a compile-time error at `P = 1;` in the resultant type
partial class C { replace int P { get { return 0; } } }
So how is that any better? You _must_ be able to add a setter in this case. :)
The current proposition doesn't transform methods at compile time, it just rename the existing methods and adds a new method which has [a possibility] to call the original one.
If you say so, "calling the original member" would make sense. But I think original(...); shouldn't be callable that way, because you can dramatically change the call site by passing any possible value to the original method parameters which is against of your argument regarding principle of least surprise. I don't think that is a good idea, replace should be able to only change the _body_ of the original and nothing else. In that case, original; (without an argument list) would be sufficient, hinting the compiler to call the original with exact same arguments passed to the method.
I expect that for the constructors there will be the same technique used (why otherwise?)
Because a constructor ain't no method, so it can't be renamed, it's able to assign readonly fields. etc. In this case original; would mean to _insert_ source constructor's body at its position.If you think that way, you can see why absence of the constructor initializer in the replaced member can be a better option rather than duplicating it.
alrz
on 26 Apr 2016
Does this feature handle 'replacing' functions / properties from libraries, or only from sources ?
vbcodec
on 27 Apr 2016
@vbcodec only sources and only within the same project. The primary use case (as i see it) is for code generation, like for replacing an auto property with a more complicated implementation or generating code versions of views in an asp.net app
aL3891
on 27 Apr 2016
It would be really nice if I could change the return type of a method in a replacement.
Toying around with #261, I feel like I could write this:
public static ValueTaskEnumerable<int, Af.Enumerator> AfYieldingStuff()
{
Af.Await(ValueTask_Foo());
Af.Yield(1);
Af.Await(ValueTask_Bar());
Af.Yield(2);
return Af.Break<ValueTaskEnumerable<int, Af.Enumerator>>();
}
and use code generators to generate a zero allocation async foreach method YieldingStuff. It would be nice though If I could instead write:
[ReplaceThis]
public static async ValueTaskEnumerable<int> YieldingStuff()
{
await ValueTask_Foo();
yield return 1;
await ValueTask_Bar();
yield return 2;
}
and generate this:
public static replace ValueTaskEnumerable<int, Cg3> YieldingStuff()
{
var machine = new Cg3();
...
}
public struct Cg3 : IAsyncStateMachine { ... }
@bbarry That is a valid request but also very dangerous. If replace can change the return type, it could easily go wrong. what if you didn't mean to change it? I think replaceable parts should be categorized to wholly (replace) and partially (partial, #9178) then you can use whichever fits your use case.
alrz
on 4 May 2016
I think it is potentially dangerous, but I am not sure it matters. You would need to do some explicit action to run a generator (import a nuget or write a generator in your code somewhere) so as the author of a project, you have full control over what generators are being run. Surely you have made an intention to change this then right?
I suppose it doesn't really matter, I could just as easily write:
[StructEnumerableCodeGen("public static", "YieldingStuff")]
private async IAsyncEnumerable<int> YieldingStuffCodeGenOriginal()
{
await ValueTask_Foo();
yield return 1;
await ValueTask_Bar();
yield return 2;
}
I would like a clear way to see that a method is being superseded.
For example.
// user written code
public partial class MyClass
{
public supersedeable void Method1()
{
// something totally awesome here
}
}
So that anyone reading the code from Method1() knows something special is going on. Please come up with a better keyword then supersedeable for it!
IanRingrose
on 17 May 2016
@IanRingrose Whenever a new feature is added to the language, some people ask for a really explicit syntax so it is obvious that "something special" is going on. But once the language feature has been in the language for a few months, the "something" is no longer "something special" and that extra syntax just becomes bothersome boilerplate.
gafter
on 17 May 2016
@IanRingrose: Also, this is primarily a tooling issue. Visual Studio could simply show an icon that indicates that the method is replaced, potentially even allowing you to jump to the replacement method with a click/shortcut.
axel-habermaier
on 17 May 2016
@axel-habermaier agree that instead of expressing it in the language it is more a tooling issue
like described here #11038.
biqas
on 17 May 2016
A partial method, has the partial keyword used on both sides, so you can see the empty method may be replaced. (Likewise virtual methods are marked in base classes, so you can see that a sub class may redefine them.)
Therefore I think it is reasonable that a method that will be superseded should likewise have a keyword on both sides, so you can see that it may be superseded.
IanRingrose
on 18 May 2016
@IanRingrose As the entire point of this feature is to provide AOP/type-provider functionality to automatically perform a lot of what would be boilerplate requiring new boilerplate to even allow the boilerplate would just be a lot of unnecessary noise. The developer is already opting into using said code generator by adding it to their project and following its conventions for decorating the target classes.
HaloFour
on 18 May 2016
Then way require the class to be marked partial at all?
It may be best just to require an Attribute to control AOP, with the Attribute enabling AOP on all code at or below the level it is put on.
The workflow of compile, run code generator, recompile (with generated code) also need considering. This has to work with ALL C# compilers not just roslyn. The code generator also must be able to be written in a way that works with all C# compilers.
IanRingrose
on 18 May 2016
@IanRingrose
Indeed, it could be argued that requiring the partial modifier on the class is also extraneous, which is why #6953 exists.
It may be best just to require an Attribute to control AOP, with the Attribute enabling AOP on all code at or below the level it is put on.
I expect most generators will rely on attributes. Requiring another attribute seems, again, like useless boilerplate. As a developer I have the choice to simply not add the generator to my project.
The workflow of compile, run code generator, recompile (with generated code) also need considering.
IIRC the generator runs as a part of the compilation? It should be a single operation.
This has to work with ALL C# compilers not just roslyn. The code generator also must be able to be written in a way that works with all C# compilers.
Sure, as long as all other C# compilers fork from Roslyn then that will be possible. Beyond that, it's simply a ludicrous expectation given that generators, like analyzers, will definitely rely heavily on Roslyn APIs and ASTs.
HaloFour
on 18 May 2016
Then just define a Roslyn API to replace a method, without any need for adding a new keyword to C# at all. (Require that the Roslyn compiler MUST ALWAYS output a information line if a "add in" modifies its output. Think of a virus that spreads by using AOP....)
IanRingrose
on 18 May 2016
@IanRingrose
There isn't an API to "replace" the method, that's just source emitted to a partial class. You can type that by hand if you wanted to. The Roslyn API, and the guts of the generator, is the analyzer which scans the code to figure out if/what additional source compilation units to inject into the current compilation.
HaloFour
on 18 May 2016
@IanRingrose
Let me back that up. There _is_ a Roslyn API, as described here:
https://github.com/dotnet/roslyn/blob/features/generators/docs/features/generators.md
That API very intentionally just allows the generator to inject additional source into the current compilation. That source _may_ decorate existing members through the use of replace but that's only one possible scenario.
I expect that Roslyn will output details regarding the generators that were included as a part of the compilation, and tools will support seeing the generated source.
HaloFour
on 18 May 2016
With regards to order of superseding when multiple replacements are used. One potential method to help alleviate this could be an attribute that hints to the compiler desired order, so something like:
//User code
public void DoSomething(int x, int y)
{
//Some code
}
//Tool code 1
[ReplaceOrder(EReplaceOrder.Early)] //Replaced as soon as possible
public replace void DoSomething(int x, int y)
{
logger.log("DoSomething() Started");
x *= 2;
original(x, y);
logger.log("DoSomething() Ended");
}
//Tool code 2
[ReplaceOrder(EReplaceOrder.Late)] //Replaced as late as possible
public replace void DoSomething(int x, int y)
{
BeginProfiling();
original(x, y);
EndProfiling();
}
//Resulting code would be
public void DoSomething(int x, int y)
{
BeginProfiling();
logger.log("DoSomething() Started");
x *= 2;
//Some code
logger.log("DoSomething() Ended");
EndProfiling();
}
Then, any methods that share the same ReplaceOrder value - however it's implemented - are evaluated and compiled in the usual lexical order. This would act purely as a hinting system so the methods that critically must wrap around all content can request that they are the last (Or nearly the last) method to be considered with replacement, while methods that perform any critical last second processing can request that they are considered for replacement first or at least as early as possible. Methods without the attribute would simply be considered 'Normal' and compile between Early and Late.
Being an attribute would negate the need to add more key words and if a compiler - for some reason - implemented superseding but not the attribute, the code could still compile fine anyway assuming the user adds a blank placeholder attribute. My use of an enum is just for example, although I feel it might be better than a numeric ordering system as multiple tools could start fighting for the highest or lowest number so they can ensure their own order (Something zindex in CSS often shows). Using an enum, they can't try to force a specific order with extremely large numbers, but they can try to get an order that suits them best.
danm36
on 28 Jun 2016
@danm36 You need a lot more fine grained control than just an enum that specifies early, late, etc. If you have 3 generators, you need to be able to easily order them, and insert a fourth into the order at a later time if necessary. I can't think of any better mechanism for this than real numbers.
MgSam
on 28 Jun 2016
I've been thinking that The order should be something that is specified by
the project. Just as references are per project, I think that if you order
that generators and source code modifiers are run at should be specified at
a project level as well.
On Tuesday, June 28, 2016, danm36 [email protected] wrote:
With regards to order of superseding when multiple tools are in use. One
potential method to help alleviate this could be an attribute that hints to
the compiler desired order, so something like://User code
public void DoSomething(int x, int y)
{
//Some code
}//Tool code 1
[ReplaceOrder(EReplaceOrder.Early)] //Replaced as soon as possible
public replace void DoSomething(int x, int y)
{
logger.log("DoSomething() Started");
x *= 2;
original(x, y);
logger.log("DoSomething() Ended");
}//Tool code 2
[ReplaceOrder(EReplaceOrder.Late)] //Replaced as late as possible
public replace void DoSomething(int x, int y)
{
BeginProfiling();
original(x, y);
EndProfiling();
}//Resulting code would be
public void DoSomething(int x, int y)
{
BeginProfiling();
logger.log("DoSomething() Started");
x *= 2;
//Some code
logger.log("DoSomething() Ended");
EndProfiling();
}Then, any methods that share the same ReplaceOrder value - however it's
implemented - are evaluated and compiled in the usual lexical order. This
would act purely as a hinting system so the methods that critically must
wrap around all content can request that they are the last (Or nearly the
last) method to be considered with replacement, while methods that perform
any critical last second processing can request that they are considered
for replacement first or at least as early as possible. Methods without the
attribute would simply be considered 'Normal' and compile between Early and
Late.Being an attribute would negate the need to add more key words and if a
compiler - for some reason - implemented superseding but not the attribute,
the code could still compile fine anyway assuming the user adds a blank
placeholder attribute. My use of an enum is just for example, although I
feel it might be better than a numeric ordering system as multiple tools
could start fighting for the highest or lowest number so they can ensure
their own order (Something zindex in CSS often shows). Using an enum, they
can't try to force a specific order with extremely large numbers, but they
can try to get an order that suits them best.—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/dotnet/roslyn/issues/5292#issuecomment-229039421, or mute
the thread
https://github.com/notifications/unsubscribe/AM-qVkvs9_bon47LoK20gxoB5O5NSZv0ks5qQRjfgaJpZM4F-yoq
.
Tony Valenti
TonyValenti
on 28 Jun 2016
Given that project files often get messed up on code merges etc, it seem high risk to depend on them for something important.
IanRingrose
on 28 Jun 2016
@MgSam Then perhaps support both, with Early == -5, Normal = 0 and Late == 5 (Or some other values with enough space between for integer replacements that are earlier than 'Late' or later than 'Early'), so that tools can choose to be loosely ordered via an enum or strictly ordered via an integer. Once ordered via this attribute, methods that share the same order value are ordered per their file name and location within the file (Or some other ordering algorithm).
danm36
on 28 Jun 2016
Once ordered via this attribute, methods that share the same order value should give an error. Anything else will make debugging too hard, as rename a file should not change the meaning of code.
IanRingrose
on 28 Jun 2016
Instead of an error, perhaps just a warning (And only if they were ordered by an integer)? The initial use case for this was primarily for tool generated code, and - depending on the tool - it could be impossible to fix as the code and/or attribute could be regenerated by the tool on the next compile. Locking out a user from compiling their code because two of their code generators use the same order value seems like a bad move. With a warning, user written code can be flagged if there is an order conflict, but tool generated code an disable the warning for their section as the tool is aware that there could be conflicts with other tool generated code. Code without the order attribute, or using one of the 'loose' order enums would be exempt from the warning as they have indicated no need for strict replacement ordering.
danm36
on 28 Jun 2016
I agree, it probably shouldn't matter if multiple replace members happened to be marked with the same priority. I assume that most generators won't care and would share the same "normal" priority, or just omit the attribute altogether, which should produce the same effect. The question is how to then ensure that the members are replaced in a deterministic order, which I would argue should be in the same order as the analyzers/generators are run, which should be in the same order as the analyzers/generators are referenced in the project.
HaloFour
on 28 Jun 2016
The intent is to have the generators run in the order they are specified to the compiler, against declarations in lexical order (with documents also ordered as they are specified to the compiler.)
The trouble is going to be communicating that order through the tools like VS solution explorer which orders documents by name, and the mechanism that adds documents to project files which may be arbitrary. Of course, by editing the project file by hand you can easily manipulate document and generator order.
mattwar
on 28 Jun 2016
I think that, perhaps, it would be better if it was exposed as a tab in the
project properties, not in the tree itself.
On Tuesday, June 28, 2016, Matt Warren [email protected] wrote:
The intent is to have the generators run in the order they are specified
to the compiler, against declarations in lexical order (with documents also
ordered as they are specified to the compiler.)The trouble is going to be communicating that order through the tools like
VS solution explorer which orders documents by name, and the mechanism that
adds documents to project files which may be arbitrary. Of course, by
editing the project file by hand you can easily manipulate document and
generator order.—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/dotnet/roslyn/issues/5292#issuecomment-229117575, or mute
the thread
https://github.com/notifications/unsubscribe/AM-qVgmRLy63nNDr4CPAtd5Vc5PnKzW6ks5qQVcbgaJpZM4F-yoq
.
Tony Valenti
TonyValenti
on 28 Jun 2016
@mattwar I don't think that is a good solution. You shouldn't have two identical sets of code files that behave differently depending on the order of compilation. There's nothing that behaves like that currently in the language (as far as I'm aware).
Also, as someone writing code I shouldn't have to look beyond the file I'm editing to wonder what the behavior of the code will be. Except in a few corner cases (overflow checking) that's true today.
Is there a reason you guys don't want to explicitly specify the order using a number? It's worked well for PostSharp, it seems odd that you'd deviate significantly from an established, successful product that already adds this sort of capability to C#.
MgSam
on 28 Jun 2016
@mgsam There are two different issue here. One is determinism, since order of application affects the output how does the compiler choose the order that the generators are run and the order of the declarations they are run against. The second is user ability to adjust that order. Having generators declare their order in some way is a means to let the user (or author in this case) adjust the order. But there has to be an order in the first place for there to be determinism.
mattwar
on 29 Jun 2016
This is similar to _import precedence_ in XSLT, except in XSLT source files are explicitly linked together via xsl:import, so the order problem doesn't exist.
The idea of using an attribute to define the order is not bad, but it should be at the partial class level, otherwise organization can get confusing and difficult to follow if different partials use different priorities for different members
Please don't use original, it has a different meaning in XSLT 3.0.
maxtoroq
on 3 Aug 2016
Hi all,
why not use an attribut on the generated side, to indicate which member is superseded? In this way, we don't have to add any new keyword in C#. And the original method and the one generated by the tool can have different name.
Something like this :
// user written code
public partial class MyClass
{
public void Method1()
{
// something totally awesome here
}
}
// tool generated code
public partial class MyClass
{
[Supersede(nameof(Method1))]
public void ToolGeneratedName()
{
Console.WriteLine(“entered Method1”);
Method1();
Consolel.WriteLine(“exited Method1”);
}
}
The "Supersede" attribut signals to the compiler that this method will replace the original "Method1". Inside the method decorated with this attribut, the name "Method1" refers to the user written code, and outside, to the method generated by the tool.
We can have an order on the attribut to help the compiler sort out the order of superseding.
This can be implement either in the compiler or in an post compilation process.
EricGarnier
on 28 Sep 2016
@EricGarnier Well, the question is whether our goal is to avoid inclusion of new keywords.
First of all, what are we going to do if Method1 is overloaded? Are we going to invent some special syntax for this case or ask the compiler to choose the method with the same signature? What is the method is an indexer or explicit interface implementation?
Next. with the attribute, the reader just sees some unintelligible name, so we are kind of lying to the reader. What if ToolGeneratedName is actually used in the class and makes ambiguity? Should the parsing rules depend so strictly on just one attribute? Must the IDE and all the tools know that the generated name is bogus, and may be ignored when e.g. the user creates another function with the duplicated name but without the attribute?
From my point of view your proposal contradicts the rule-of-thumb "attributes can be roughly ignored when one reads the code"; you propose a very deep change of the language expressive means just for avoiding a single new keyword.
vladd
on 30 Sep 2016
Decomposing the original to return new types or rearrange parameters would feel better through a limited macro system. For tooling, just getting extra code slices is enough. Here are keyword names I like. There probably should generator entries in the .xxproj file read (FIFO) and if not then msbuild should get an explicit listing of files for compilation. Attributes and source magic number ordering is too constraining.
// user written code
public partial class MyClass
{
public augment string Property1 {get;} = "awesome";
public augment void Method1()
{
// something totally awesome here
}
}
// tool #1 generated code
public partial class MyClass
{
public transform void Method1()
{
// do something interesting
primary();
}
}
// tool #2 generated code
public partial class MyClass
{
public transform string Property1()
{ get =>{
// do something else interesting
return primary;}
}
}
pr-yemibedu
on 23 Nov 2016
This proposal is now being tracked at https://github.com/dotnet/csharplang/issues/107
gafter
on 24 Mar 2017
Related issues
marler8997
·
3Comments
SolalPirelli
·
3Comments
MadsTorgersen
·
3Comments
johndog
·
3Comments
codingonHP
·
3Comments
Most helpful comment
@IanRingrose Whenever a new feature is added to the language, some people ask for a really explicit syntax so it is obvious that "something special" is going on. But once the language feature has been in the language for a few months, the "something" is no longer "something special" and that extra syntax just becomes bothersome boilerplate.