Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Champion "Allow Generic Attributes" #124

Open
4 of 5 tasks
gafter opened this issue Feb 15, 2017 · 60 comments
Open
4 of 5 tasks

Champion "Allow Generic Attributes" #124

gafter opened this issue Feb 15, 2017 · 60 comments
Assignees
Labels
Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification Proposal champion Proposal
Milestone

Comments

@gafter
Copy link
Member

gafter commented Feb 15, 2017

See

@gafter gafter changed the title Champion Allow Generic Attributes Champion "Allow Generic Attributes" Feb 21, 2017
@gafter gafter added this to the X.0 candidate milestone Feb 22, 2017
@AlphaDriveLegal
Copy link

I've wanted this for a long time!

@Mafii
Copy link

Mafii commented Mar 14, 2017

I would be happpy to have this, too!

@weitzhandler
Copy link

weitzhandler commented Aug 4, 2017

Very compelling.

Here's my use case, one among many:

public class ValidateIf<TValidationAttribute> : ValidationAttribute
{
  public TValidationAttribute Attribute { get; private set; }
  //other properties to resemble condition
  public ValidateIf(Action<TValidationAttribute> configure)
  {
    Attribute = configure();
  }
  protected override ValidationResult IsValid(object value, ValidationContext validationContext)
  {
    var shouldValidate = this.ShouldValidate();
    if(!shouldValidate) return ValidationResult.Success;
    else return Attribute.GetValidationResult(value, validationContext);
  }
}

Usage:

public class Person
{
  [ValidateIf<RangeAttribute>(() => new RangeAttribute(params), Condition set up...)]
  public string Name { get; set; }
}

Short in time now, sorry for the poor pseudo, but I'm sure I'm understood.

@Joe4evr
Copy link
Contributor

Joe4evr commented Aug 4, 2017

@weitzhandler Your example is pretty flawed since lambdas can't be passed as an attribute parameter; only primitives, string, typeof() expressions, and DateTime literals (VB).

A more sensible example of where a generic attribute would be useful:

// This concerns a bot command system that can parse strings into typed arguments,
// but sometimes the "default" reader for something may be too narrow,
// so this on a parameter would override which class does the reading for that argument.
[AttributeUsage(AttributeTargets.Parameter)]
public sealed class OverrideTypeReaderAttribute<TReader> : Attribute
    where TReader : TypeReader
{
    // We're only interested in the Type info at this point,
    // but we would really like the type-safety of the generic constraint
    public Type TypeReaderType { get; }

    public OverrideTypeReaderAttribute()
    {
        TypeReaderType = typeof(TReader);
    }
}
// Usage:
[Command("foo")]
public Task Foo([OverrideTypeReader<AlternativeBoolReader>] bool switch)
{
    //.....
}

Currently we imitate the constraint by taking a Type parameter in the constructor and checking IsAssignableFrom() at runtime, but pushing the check to compile-time via constraint would be neat and save us from needing to throw an exception if it fails.

@weitzhandler
Copy link

@Joe4evr
I thought part of this proposal was allowing complex params I attribute initialization, which I saw in several issues, maybe in the Roslyn repo.
And that's why I was going for an example that utilizes both, and I do vote for both genetic attributes and better and more flexible attribute initialization and setup options.

@Joe4evr
Copy link
Contributor

Joe4evr commented Aug 5, 2017

I thought part of this proposal was allowing complex params I attribute initialization

I have not seen this proposal before, not to mention that such a thing would require quite a hefty spec change since as it stands, the compiler needs to be able to serialize the arguments directly in the metadata.

It is possible, if wanted for whatever reason, to instantiate an attribute as just a normal class, and that could use any type of parameter:

var attr = new SomethingSpecialAttribute(GetSomeData());

But that should only be a secondary constructor, otherwise it's totally useless to have that class be an attribute in the first place. (I have an example of where this is used in the same codebase I described above, but that's getting off-topic.)

@gafter
Copy link
Member Author

gafter commented Oct 28, 2017

I thought part of this proposal was allowing complex params I attribute initialization,

No.

@alrz
Copy link
Member

alrz commented Jan 13, 2018

We need a mechanism to understand which target runtime it works on. We need that for many things, and are currently looking at that. Until then, we can't take it.

I suppose that's on the use-site, whereas an abstract generic attribute doesn't seem to be harmful,

abstract class SomeAttribute<T> : Attritbue {}

Could we permit the above without (any) runtime support?

@ymassad
Copy link

ymassad commented Dec 5, 2018

Would this feature support this?

public class GenericClass<T>
{
    [SomeAttribute<T>]
    public void DoSomething(T input)
    {
    }
}

public class SomeAttribute<T> : Attribute
{

}

@roydukkey
Copy link

@ymassad According to the proposal, your example is correct but possibly not for the reasons that you expect. Here is an example that might make things a little clearer.

[SomeAttribute<A>]
public class GenericClass<A>
{
	[SomeAttribute<B>]
	public void DoSomething(B input) { }
}

public class SomeAttribute<C> : Attribute
{
	// `C` is either `A` or `B` according to its target.
}

As you can see from the example, the generic context comes from the attributes target.

@ymassad
Copy link

ymassad commented Dec 6, 2018

@roydukkey , I am not sure I understand. Can you elaborate?

Is B a concrete type?

@roydukkey
Copy link

@ymassad Sorry. I meant to write this.

[SomeAttribute<B>]
public void DoSomething<B>(B input) { }

I believe this is the spec.

@AviAvni
Copy link

AviAvni commented Dec 6, 2018

The attribute can't have open generic type it must be instantiated at compile time

@jnm2
Copy link
Contributor

jnm2 commented Dec 6, 2018

@ymassad What attribute instance would you expect typeof(GenericClass<>).GetMethod("DoSomething").GetCustomAttributes() to contain in your example?

@ymassad
Copy link

ymassad commented Dec 6, 2018

@jnm2 ,

Thanks. I see now why this is not possible.

what about this:

public class GenericClass<T>
{
    [SomeAttribute(typeof(T)]
    public void DoSomething(T input)
    {
    }
}

public class SomeAttribute : Attribute
{
    public SomeAttribute(Type type)
    {
        this.Type = type;
    }

    public Type Type {get;}
}

typeof(GenericClass<>).GetMethod("DoSomething").GetCustomAttributes() could return a SomeAttribute instance with Type containing the parameter type T.

@jnm2
Copy link
Contributor

jnm2 commented Dec 6, 2018

@ymassad The problem there (if I remember the metadata format correctly) is that attribute arguments of type System.Type are stored as serialized strings containing the assembly-qualified name. What string can be deserialized (similar to Type.GetType) in such a way that it resolves to a generic type parameter? I think this would require a runtime change.

Edit: !0 (referring to the generic type parameter with index 0) and !!0 (same but for generic method parameters) are how this would work.

@jnm2
Copy link
Contributor

jnm2 commented Dec 6, 2018

Yep, https://www.ecma-international.org/publications/files/ECMA-ST/ECMA-335.pdf, section II.23.3 Custom attributes, page 268:

  • If the parameter kind is System.Type, (also, the middle line in above diagram) its value is stored as a SerString (as defined in the previous paragraph), representing its canonical name. The canonical name is its full type name, followed optionally by the assembly where it is defined, its version, culture and public-key-token. If the assembly name is omitted, the CLI looks first in the current assembly, and then in the system library (mscorlib); in these two special cases, it is permitted to omit the assembly-name, version, culture and public-key-token.

@ymassad
Copy link

ymassad commented Dec 6, 2018

@jnm2 , thanks.

@HaloFour
Copy link
Contributor

HaloFour commented Aug 6, 2021

@Shadowblitz16

the issue is disallowing anything that is not a component to be a compile time error.

Right, if you want to use generic type constraints then you'll need to define multiple versions of that attribute, one for each number of generic type arguments you can accept:

SharpLab

One can argue that variadic generics would make your use case easier, but it gets substantially more complicated for any use case that would want to use those generic type arguments for anything other than reflection purposes.

@Shadowblitz16
Copy link

Shadowblitz16 commented Aug 6, 2021

@HaloFour this would work very well with variadic generic paramaters.
still now sure how it would be passed to a generic array though

@RikkiGibson
Copy link
Contributor

dotnet/roslyn#55577

We will make this feature available in the "preview" language version in .NET 6 RC1. We found that a large number of runtimes and tools (Mono, C++/CLI, etc.) may still have issues consuming generic attributes, so we are going to take an additional release cycle to try and iron out the issues, and give ourselves some time to make breaking changes with runtime APIs.

For example, if a user calls CustomAttributeData.GetCustomAttributes(someType, typeof(GenericAttr<>)) there is an open question about whether it should give all the constructions of the attribute which were used on the type. dotnet/runtime#56887

using System;

// what should the value of 'attrs' be here?
var attrs = CustomAttributeData.GetCustomAttributes(typeof(C), typeof(Attr<>));

[AttributeUsage(AttributeTargets.All, AllowMultiple = true)]
class Attr<T> : Attribute { }

[Attr<int>]
[Attr<string>]
class C { }

@johnscott999
Copy link

It’s a bit cheeky, but why not Attribute[]

Attribute[] attrs = //…
attrs[0].GetType() // => typeof(Attr<int>)

from there you can get what you need with reflection.

practically I’d probably, inherit from a non generic base type that exposed the generic types of derived as properties, but that’s a design decision.

as for allow multiple, I think you’ve got to treat each different generic as a different class, the same way you would static properties in a generic class. I’d support adding an attribute usage for AllowMultipleGeneric (name pending), that would only allow one Attr<T> decorator if set. That would cover all the use cases that come to mind

@Joe4evr
Copy link
Contributor

Joe4evr commented Aug 13, 2021

practically I’d probably, inherit from a non generic base type that exposed the generic types of derived as properties

I have a feeling that's going to be the majority case anyway.

// what should the value of 'attrs' be here?

Looking at the existing overloads for CustomAttributeData.GetCustomAttributes, they all return an IList<CustomAttributeData> so I see no reason to deviate from that. If called with an open/half-open type, I'd expect it to contain the data of all the attributes applied with the "missing" type arguments filled in to what they are on the use-site, or an empty collection if none matched.

[AttributeUsage(AttributeTargets.All, AllowMultiple = true)]
class Attr<T1, T2> : Attribute { }

[Attr<long, int>]
[Attr<long, string>]
[Attr<int, string>]
[Attr<object, string>]
class C { }

// { Attr<long, int>, Attr<long, string>, Attr<int, string>, Attr<object, string> }
var attrs0 = CustomAttributeData.GetCustomAttributes(typeof(C), typeof(Attr<,>));

// { Attr<long, int>, Attr<long, string> }
var attrs1 = CustomAttributeData.GetCustomAttributes(typeof(C), typeof(Attr<long,>));

// { Attr<long, string>, Attr<int, string>, Attr<object, string> }
var attrs2 = CustomAttributeData.GetCustomAttributes(typeof(C), typeof(Attr<,string>));

// { } (none of the applied attributes have 'object' as T2)
var attrs3 = CustomAttributeData.GetCustomAttributes(typeof(C), typeof(Attr<,object>));

Does that make sense?

I think it may be beneficial to add a new TypeArguments property to CustomAttributeData as well, even though they can also simply be fished out of AttributeType, it could be more discoverable for people already working with this a lot.

@Rekkonnect
Copy link
Contributor

The catch here is that you cannot partially bind a generic type in a typeof expression; and I can't remember if you're allowed to not fully bind an open generic type via reflection. I can see why partial generic binding shouldn't be prohibited, and I support it.

as for allow multiple, I think you’ve got to treat each different generic as a different class, the same way you would static properties in a generic class. I’d support adding an attribute usage for AllowMultipleGeneric (name pending), that would only allow one Attr<T> decorator if set. That would cover all the use cases that come to mind

#124 (comment)


Now, onto the real stuff:

  • Generic types are immediately ignored on certain implementations of GetCustomAttributes. That's because up until now, attributes couldn't be generic. This would have to change accordingly after generic attributes go live.
  • Statically typed open generic types aren't usable outside of typeof contexts, meaning you'd have to bind the type arguments of a generic attribute instance.

Those arguments are enough to show that an open generic attribute type can function as a wildcard to return all attributes that are of the type.

The other option is to discard that option and throw an exception. I see no reason to invalidate functionality that cannot be otherwise utilized. It should however be properly marked down in the documentation that open generic attribute types result in all attribute instances binding to the specified generic type.

@TheFanatr
Copy link

I totally +1 the idea of allowing partial generic binding to be a thing, but why stop at typeof? If static generic type reference syntax needs to be improved to support constraining to specific parameters, via relaxing rules around generic parameters, I'd say that this should be done more broadly and not focus on just this specific use of generic syntax.

Statically typed open generic types aren't usable outside of typeof contexts, meaning you'd have to bind the type arguments of a generic attribute instance.

@alfasgd Is there some technical reason behind this (beyond the obvious)?

I understand that in most cases the generic type would need to be interacted with (so not talking about trivial use of is) and so would need to have all the types resolved to use the full type surface in non-trivial cases, but this can be worked around by not allowing access to members which expose undefined generic parameters and/or exposing the least derived compatible type based on the variance of the parameters.

More than this just being convenient, the is syntax was initially basically meant to replace and make (...).GetType() == typeof(...) more optimizable anyways, so if you can use open generics in typeof, then it would make sense to allow them, at the bare minimum, in the trivial is syntax, but I'd personally aim a little higher. To be clear, while I understand it is not possible to constrain the earlier-mentioned type check that uses == with the proposed changes to partial generics syntax, it would still be possible using the reflection APIs above in the thread, so it seems like a reasonable extension of the syntax sugar for is, additionally considering that GetType and typeof can be considered reflection, and that this is all still static (not dynamic) if variance-based inference is used as suggested to fill in the unspecified parameters and maybe with other solutions as well.

Let's take an example.

Object table = new Dictionary<string, bool> { ["e"] = true };

if (table is Dictionary<string,> referenceTable) {
    Console.WriteLine(referenceTable.ContainsKey("e"));
}

if (table is Dictionary<,bool> referenceTable) {
    Console.WriteLine(referenceTable.Values[0]);
}

With that, it would seem reasonable to then also enable similar access to static members, although I recognize it is a little different.

static class TestClassA<T> {
    string testA = "";
}

...

TestClassA<>.testA = ...;

And I mean, while I'm at it, may as well point out this one too. (Not open generic type, but still a partial generic)

public static TNotInferrable TestA<TInferrableA, TInferrableB, TNotInferrable>(TInferrableA inferrableA, TInferrableB inferrableB) => ...;
public static void TestB<TInferrableA, TInferrableB>(TInferrableA inferrableA, TInferrableB inferrableB) => ...;

...

var success = TestB<,,bool>("", 341.56D);
TestA("", 341.56D); // currently legal

Basically if you're going to relax restrictions on generic syntax to allow constraining by specific parameters for the sake of convenience, may as well apply that wherever possible.

Just some thoughts. Sorry for the messy writing, late at night (early in the morning).

@Rekkonnect
Copy link
Contributor

That's an already open proposal, you can find it by looking for "unbound", "partially bound", "partially open" generics

@seanblue
Copy link

Is it possible for generic attributes to work on .NET Framework? In the past, some "unsupported" features have become compatible just by copying a few types (e.g. init keyword) and other times it's impossible (as far as I know). While I am able to define generic attribute types and apply them to classes/methods/etc., the methods for actually finding the attributes used (e.g. MemberInfo.GetCustomAttributes) throw an exception when passing in a generic attribute. Are there any types I can copy or otherwise anything I can do to fully enable this feature on .NET Framework?

@hez2010
Copy link

hez2010 commented Dec 13, 2022

Are there any types I can copy or otherwise anything I can do to fully enable this feature on .NET Framework?

Nope. The generic attributes feature requires a runtime change and the change won't be backported to .NET Framework.

@seanblue
Copy link

@hez2010 I had a feeling that would be the case. Thanks for confirming. 🙂

@BlinD-HuNTeR
Copy link

@seanblue It should be possible, however, to write something that directly reads and parses metadata from the assembly file, without using the reflection infrastructure. Probably something based off of Mono.Cecil would do the job, depending on what you want.

@TonyValenti
Copy link

TonyValenti commented Dec 14, 2022

In the past, to get around this, I will have a generic attribute base class and then create a non-generic attribute that inherits from it and use that.

I also use a similar approach when I want to provide values to an attribute that are classes/disallowed types.

ie:

public class MyAttribute<T> : Attribute { }
public class MyStringAttribute : MyAttribute<string> { } 

[MyStringAttribute]
public class Foo { }

@seanblue
Copy link

@BlinD-HuNTeR I don't have an immediate use case for generic attributes anyway. I was just updating to C# 11 and trying to see what worked.

@TonyValenti I came to the same conclusion. As long as you only use a non-generic subclass of the generic attribute, everything seems to work fine. Not ideal, but still better than nothing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification Proposal champion Proposal
Projects
None yet
Development

No branches or pull requests