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

Cannot read attribute arguments from metadata when the original definition parameter is of a type parameter type #55190

Closed
RikkiGibson opened this issue Jul 28, 2021 · 2 comments · Fixed by #70151
Assignees
Milestone

Comments

@RikkiGibson
Copy link
Contributor

RikkiGibson commented Jul 28, 2021

Version Used: features/generic-attributes

Steps to Reproduce:

[Fact, WorkItem(55190, "https://github.com/dotnet/roslyn/issues/55190")]
public void GenericAttributeParameter_01()
{
var source = @"
using System;
class Attr<T> : Attribute { public Attr(T param) { } }
[Attr<string>(""a"")]
class Program { }
";
var verifier = CompileAndVerify(source, sourceSymbolValidator: verify, symbolValidator: verifyMetadata);
verifier.VerifyTypeIL("Program", @"
.class private auto ansi beforefieldinit Program
extends [netstandard]System.Object
{
.custom instance void class Attr`1<string>::.ctor(!0) = (
01 00 01 61 00 00
)
// Methods
.method public hidebysig specialname rtspecialname
instance void .ctor () cil managed
{
// Method begins at RVA 0x2058
// Code size 7 (0x7)
.maxstack 8
IL_0000: ldarg.0
IL_0001: call instance void [netstandard]System.Object::.ctor()
IL_0006: ret
} // end of method Program::.ctor
} // end of class Program
");
void verify(ModuleSymbol module)
{
var program = module.GlobalNamespace.GetMember<TypeSymbol>("Program");
var attrs = program.GetAttributes();
Assert.Equal(new[] { "Attr<System.String>(\"a\")" }, GetAttributeStrings(attrs));
}
void verifyMetadata(ModuleSymbol module)
{
var program = module.GlobalNamespace.GetMember<TypeSymbol>("Program");
var attrs = program.GetAttributes();
Assert.Equal(new[] { "Attr<System.String>" }, GetAttributeStrings(attrs));
}
}

Expected Behavior: If we reference Program from metadata and get a symbol for it, we should be able to GetAttributes() and find an attribute with an argument "a" on it.

Actual Behavior: We get an attribute with no arguments.

Related to #36285

@RikkiGibson
Copy link
Contributor Author

RikkiGibson commented Jul 28, 2021

There are really two issues here:

  1. (blocking stable release of the feature) reflection at runtime doesn't work in some cases. Fixed in Cannot reflect on generic attributes whose constructors original definition uses type parameters runtime#56492
  2. (not blocking) reading attribute arguments from metadata in the compiler doesn't work.

For (2) I think it is fine to just fix down the line because obtaining arguments for user-defined attributes from metadata isn't that common of a scenario. For example, analyzers which are interested in specific attributes are only interested in usages of that attribute in source.

For (1) it seems like a severe enough issue to block stable release of the feature. However, if we can confirm both of the following, it may be reasonable to ship it under the Preview LangVersion:

  1. The compiler is emitting correct metadata in this scenario
  2. The underlying issue which prevents reflection and reading with MetadataReader will be addressed.

@jcouv

@RikkiGibson
Copy link
Contributor Author

I tried running a relevant unit test under a nightly .NET 6 runtime and I didn't see issue (2) fixed. I suspect there is still either a problem in System.Reflection.Metadata or the way we are using it.

@RikkiGibson RikkiGibson modified the milestones: 17.0, 17.1 Aug 20, 2021
@RikkiGibson RikkiGibson modified the milestones: 17.1, 17.2 Mar 11, 2022
@jcouv jcouv modified the milestones: 17.2, 17.3 May 14, 2022
@jaredpar jaredpar modified the milestones: 17.3, Backlog Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants