-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Fix CustomAttribute.DecodeValue to handle generic type instantiations #123879
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
Conversation
…t case Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
|
@copilot fix the test failure: |
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
... Fixed in 7fcb71c. The issue was that when decoding a generic type instance (e.g., |
|
@copilot test failure: |
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
... Fixed in 18d89a8. The test was incorrectly expecting the primitive type "int32" instead of the enum type name. For enum types (even those nested in generic types), the Type field contains the enum's type name (e.g., "CustomAttributeDecoderTests/GenericClassForEnum`1/E"), not the underlying primitive type. Updated the test to use Contains/EndsWith assertions to verify the type name. |
| EntityHandle genericHandle = signatureReader.ReadTypeHandle(); | ||
| int genericArgumentCount = signatureReader.ReadCompressedInteger(); | ||
| for (int i = 0; i < genericArgumentCount; i++) | ||
| { | ||
| SkipType(ref signatureReader); | ||
| } | ||
|
|
||
| info.Type = GetTypeFromHandle(genericHandle); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not correct - we're losing type information that the end user might want to interrogate. The user can do this:
var v = reader.GetCustomAttribute(attr).DecodeValue(new CustomAttributeTypeProvider());
_ = v.FixedArguments[0].Type;
And then the .Type obtained by the user would be the .Type we populate here. However, we're losing type information about generic arguments.
We don't have means to provide type information about generic arguments because the interface through which we communicate type information (ICustomAttributeTypeProvider<T>) only deals with SZArrays. No other constructed types are expressible.
I think we need new APIs (several) on ICustomAttributeTypeProvider. We could in theory default-implement these to throw BadImageFormat.
Alternative option is to construct a string representation of the type and use the existing string-based API on ICustomAttributeTypeProvider. However, not all types have string representation (function pointers, modifiers), so we'd still be losing type information.
Description
CustomAttributeDecoder.DecodeFixedArgumentTypethrowsBadImageFormatExceptionwhen encountering generic type instantiations in custom attribute signatures, specifically for enums nested in generic types:Changes
CustomAttributeDecoder.cs
SignatureTypeCode.GenericTypeInstancecase inDecodeFixedArgumentTypeSystem.Typeor enum viaGetUnderlyingEnumTypeCustomAttributeDecoderTests.cs
TestCustomAttributeDecoderGenericEnumtest withGenericClassForEnum<T>and nested enum#if NET && !TARGET_BROWSERblock matching other generic attribute testsCustomAttributeTypeProvider.GetUnderlyingEnumTypeto handle generic type instances where the enum is nested in a generic type (handles type strings like"GenericClassForEnum1/E"that cannot be resolved viaType.GetType()`)"...GenericClassForEnum1/E"`) rather than primitive type representationTesting
Validated with the exact repro from the issue - no longer throws
BadImageFormatException.Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.