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

System.Reflection.Metadata doesn't handle generic attributes #58073

Closed
Tracked by #44655
MichalStrehovsky opened this issue Aug 24, 2021 · 4 comments · Fixed by #72561
Closed
Tracked by #44655

System.Reflection.Metadata doesn't handle generic attributes #58073

MichalStrehovsky opened this issue Aug 24, 2021 · 4 comments · Fixed by #72561

Comments

@MichalStrehovsky
Copy link
Member

We get a BadImageFormatException if the custom attribute constructor refers to the T's of the owning type.

I have a fix in #57466 but I won't have time to write tests anytime soon. It's better if that's handled by the owners of S.R.M.

@krwq
Copy link
Member

krwq commented Aug 25, 2021

Proactively adding to 6.0 - per @RikkiGibson and @tmat generic attributes will ship in 6.0 which means this scenario will be broken. The initial version of the implementation is in the PR above, missing tests.

@krwq krwq removed the untriaged New issue has not been triaged by the area owner label Aug 25, 2021
@krwq krwq self-assigned this Aug 26, 2021
@joperezr
Copy link
Member

joperezr commented Sep 8, 2021

Looks like the PR you pointed to was closed. @tmat @RikkiGibson do we still want to get this in 6.0? is somebody going to finish the work that @MichalStrehovsky started?

@RikkiGibson
Copy link
Contributor

We are including the feature under the "preview" language version, which means it is not supported and it's expected that some things will still be broken in the upcoming .NET 6 release.

If we are able to get more fixes related to the feature into .NET 6, then great, but it's not urgent right now.

@joperezr
Copy link
Member

joperezr commented Sep 9, 2021

Got it, will mark this issue as net7.0 then for now. If we can still get it for 6.0 that would be great, but given the time window that seems like it could not make it in time.

@joperezr joperezr modified the milestones: 6.0.0, 7.0.0 Sep 9, 2021
@steveharter steveharter assigned buyaa-n and unassigned krwq Jul 13, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 20, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 9, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants