Skip to content

C# 11: Generic attributes #11814

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

Merged
merged 8 commits into from
Jan 19, 2023

Conversation

michaelnebel
Copy link
Contributor

@michaelnebel michaelnebel commented Jan 4, 2023

It turns out that the extractor supports generic attributes out of the box due to the existing reference design between attributes and types.
That is, we only need to implement some minor Code QL library functionality for generic attributes to fetch type arguments and the like.
However, as a part of the process @tamasvajk discovered that attributes on methods in CIL was not correctly extracted. With the help of @tamasvajk this is fixed in #11907

@michaelnebel michaelnebel marked this pull request as ready for review January 4, 2023 16:28
@michaelnebel michaelnebel requested a review from a team as a code owner January 4, 2023 16:28
@tamasvajk
Copy link
Contributor

@michaelnebel It might be worth checking if the CIL extractor also supports generic attributes. We do some attribute processing in CustomAttributeDecoder.

@michaelnebel michaelnebel changed the title C#: Generic attributes C# 11: Generic attributes Jan 10, 2023
@michaelnebel michaelnebel force-pushed the csharp/genericattributes branch 2 times, most recently from 990a7ca to 25019eb Compare January 17, 2023 15:03
@michaelnebel michaelnebel marked this pull request as draft January 17, 2023 15:04
@michaelnebel michaelnebel force-pushed the csharp/genericattributes branch from 25019eb to f506ffb Compare January 17, 2023 16:19
@michaelnebel
Copy link
Contributor Author

@michaelnebel It might be worth checking if the CIL extractor also supports generic attributes. We do some attribute processing in CustomAttributeDecoder.

After the changes to make support for attributes on methods, I can confirm that generic attributes also works out of the box for attributes extracted from CIL.
To align a bit more with source code extracted attributes, I have changed the printing of constructed types extracted from CIL.

@michaelnebel michaelnebel force-pushed the csharp/genericattributes branch from f506ffb to 2772a64 Compare January 18, 2023 07:15
tamasvajk
tamasvajk previously approved these changes Jan 18, 2023
@tamasvajk tamasvajk self-requested a review January 18, 2023 08:34
@tamasvajk
Copy link
Contributor

Let's check with DCA too. Otherwise it looks good to me.

@michaelnebel
Copy link
Contributor Author

Let's check with DCA too. Otherwise it looks good to me.

👍 Yes definitely! I will take this PR out of draft and rebase as soon as its dependency has been merged and then run DCA!

@michaelnebel
Copy link
Contributor Author

DCA looks good!
@tamasvajk : Can you re-approve. The only changes since list time is a rebase and a merge of two commits containing the change-note.

@michaelnebel michaelnebel marked this pull request as ready for review January 19, 2023 06:34
@michaelnebel michaelnebel merged commit e6aebd9 into github:main Jan 19, 2023
@michaelnebel michaelnebel deleted the csharp/genericattributes branch January 19, 2023 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants