[pedrobsaila]

Fixes #110412
Fixes #119490

Sorry for the delay, I've been busy these last weeks

[pedrobsaila]

Hey @AaronRobinsonMSFT yes you can fold in your changes, no worries about that.

[AaronRobinsonMSFT]

Hey @AaronRobinsonMSFT yes you can fold in your changes, no worries about that.

@pedrobsaila I don't think I can push to your branch. I can create a PR from my branch or you can cherry-pick my changes, up to you.

[AaronRobinsonMSFT]

/ba-g Known issues.

Issues fixed in #125051

[AaronRobinsonMSFT]

@MichalStrehovsky Please take another look at this. I've narrowed the scenario considerably.

[MichalStrehovsky]

Is the difference in exception type cause by the:

                                RuntimePropertyInfo? property = (RuntimePropertyInfo?)(type is null ?
                                    baseAttributeType.GetProperty(name) :
                                    baseAttributeType.GetProperty(name, type, []));

on the JIT side and

                        PropertyInfo? propertyInfo = walk.GetProperty(name, BindingFlags.Public | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly);

on the AOT side?

Could this be reconciled by adding type information to the GetProperty on the native AOT side too?

[MichalStrehovsky]

Trying it, worst case we revert it and accept the difference. Best case we update the test because the behavior will now be same.

[AaronRobinsonMSFT]

@MichalStrehovsky It looks like there is now a new failure for native AOT. Thoughts? I can also revert the changes and we can live with it for now.

[FAIL] System.Reflection.Tests.CustomAttributeTests.BoxedEnumAttributes
System.Reflection.CustomAttributeFormatException : 'B1' property specified was not found.
   at Internal.Reflection.Extensions.NonPortable.CustomAttributeInstantiator.Instantiate(CustomAttributeData) in /_/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Reflection/Extensions/NonPortable/CustomAttributeInstantiator.cs:line 131
   at System.Reflection.CustomAttributeExtensions.GetCustomAttribute[T](MemberInfo) in /_/src/libraries/System.Private.CoreLib/src/System/Reflection/CustomAttributeExtensions.cs:line 38
   at System.Reflection.Tests.CustomAttributeTests.BoxedEnumAttributes() in /_/src/libraries/System.Runtime/tests/System.Reflection.Tests/CustomAttributeTests.cs:line 261
   at System.Reflection.DynamicInvokeInfo.Invoke(Object, IntPtr, Object[], BinderBundle, Boolean) in /_/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/DynamicInvokeInfo.cs:line 230
[SKIP] System.Reflection.Tests.ParameterInfoTests.GetCustomAttributesDataOnParameterWithNullMetadataTokenReturnsEmptyList

[MichalStrehovsky]

@MichalStrehovsky It looks like there is now a new failure for native AOT. Thoughts? I can also revert the chances and we can live with it for now.

Do you have any thoughts on failure modes of these with CoreCLR:

using System;

RunWithLog(() => typeof(C1).GetCustomAttributes(false));
RunWithLog(() => typeof(C2).GetCustomAttributes(false));
RunWithLog(() => typeof(C3).GetCustomAttributes(false));

static void RunWithLog(Action a)
{
    try
    {
        a();
    }
    catch (Exception ex)
    {
        Console.WriteLine("*******************************************");
        Console.WriteLine(ex.Message);
        Console.WriteLine(ex.StackTrace);
    }
}


public class BaseAttributeWithTypeProp : Attribute
{
    public virtual object Prop
    {
        get;
        set;
    }
}

public class DerivedAttributeWithObjectProp : BaseAttributeWithTypeProp
{
    public override Type Prop
    {
        get => (Type)base.Prop;
    }
}

[DerivedAttributeWithObjectProp(Prop = typeof(object))]
class C1;

[DerivedAttributeWithObjectProp(Prop = "Hello")]
class C2;

[DerivedAttributeWithObjectProp(Prop = (AttributeTargets)0)]
class C3;

On .NET 10 this will print:

*******************************************
'Prop' property specified was not found.
   at System.Reflection.CustomAttribute.AddCustomAttributes(ListBuilder`1& attributes, RuntimeModule decoratedModule, Int32 decoratedMetadataToken, RuntimeType attributeFilterType, Boolean mustBeInheritable, ListBuilder`1 derivedAttributes)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeModule decoratedModule, Int32 decoratedMetadataToken, Int32 pcaCount, RuntimeType attributeFilterType)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeType type, RuntimeType caType, Boolean inherit)
   at Program.<>c.<<Main>$>b__0_0() in C:\Users\Michal\source\repos\ConsoleApp1\ConsoleApp1\Program.cs:line 3
   at Program.<<Main>$>g__RunWithLog|0_3(Action a) in C:\Users\Michal\source\repos\ConsoleApp1\ConsoleApp1\Program.cs:line 11
*******************************************
Could not resolve type 'Hello' in assembly 'ConsoleApp1, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null'.
   at System.Reflection.TypeNameResolver.GetTypeFromDefaultAssemblies(String typeName, ReadOnlySpan`1 nestedTypeNames, TypeName parsedName)
   at System.Reflection.TypeNameResolver.GetType(String escapedTypeName, ReadOnlySpan`1 nestedTypeNames, TypeName parsedName)
   at System.Reflection.TypeNameResolver.GetSimpleType(TypeName typeName)
   at System.Reflection.TypeNameResolver.Resolve(TypeName typeName)
   at System.Reflection.TypeNameResolver.GetTypeHelper(ReadOnlySpan`1 typeName, RuntimeAssembly requestingAssembly, Boolean throwOnError, Boolean requireAssemblyQualifiedName, IntPtr unsafeAccessorMethod)
   at System.Reflection.TypeNameResolver.GetTypeHelper(Char* pTypeName, RuntimeAssembly requestingAssembly, Boolean throwOnError, Boolean requireAssemblyQualifiedName, IntPtr unsafeAccessorMethod)
   at System.Reflection.CustomAttribute.GetPropertyOrFieldData(RuntimeModule module, IntPtr& blobStart, IntPtr blobEnd, String& name, Boolean& isProperty, RuntimeType& type, Object& value)
   at System.Reflection.CustomAttribute.AddCustomAttributes(ListBuilder`1& attributes, RuntimeModule decoratedModule, Int32 decoratedMetadataToken, RuntimeType attributeFilterType, Boolean mustBeInheritable, ListBuilder`1 derivedAttributes)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeModule decoratedModule, Int32 decoratedMetadataToken, Int32 pcaCount, RuntimeType attributeFilterType)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeType type, RuntimeType caType, Boolean inherit)
   at Program.<>c.<<Main>$>b__0_1() in C:\Users\Michal\source\repos\ConsoleApp1\ConsoleApp1\Program.cs:line 4
   at Program.<<Main>$>g__RunWithLog|0_3(Action a) in C:\Users\Michal\source\repos\ConsoleApp1\ConsoleApp1\Program.cs:line 11
*******************************************
Binary format of the specified custom attribute was invalid.
   at System.Reflection.CustomAttribute.GetPropertyOrFieldData(RuntimeModule module, IntPtr& blobStart, IntPtr blobEnd, String& name, Boolean& isProperty, RuntimeType& type, Object& value)
   at System.Reflection.CustomAttribute.AddCustomAttributes(ListBuilder`1& attributes, RuntimeModule decoratedModule, Int32 decoratedMetadataToken, RuntimeType attributeFilterType, Boolean mustBeInheritable, ListBuilder`1 derivedAttributes)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeModule decoratedModule, Int32 decoratedMetadataToken, Int32 pcaCount, RuntimeType attributeFilterType)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeType type, RuntimeType caType, Boolean inherit)
   at Program.<>c.<<Main>$>b__0_2() in C:\Users\Michal\source\repos\ConsoleApp1\ConsoleApp1\Program.cs:line 5
   at Program.<<Main>$>g__RunWithLog|0_3(Action a) in C:\Users\Michal\source\repos\ConsoleApp1\ConsoleApp1\Program.cs:line 11

I assume the first one might behave differently with this fix, but I don't believe the rest will be affected.

This might be a game that is better not to play. We should ask Roslyn to block this at the compiler level. Then whether we have a sensible failure mode or behaviors is a different question. We certainly won't be writing C# tests for this anymore.

[MichalStrehovsky]

Another thing that occurred to me to test:

dynamic dd = new DerivedAttributeWithObjectProp();
dd.Prop = null;

public class BaseAttributeWithTypeProp : Attribute
{
    public virtual object Prop
    {
        get;
        set;
    }
}

public class DerivedAttributeWithObjectProp : BaseAttributeWithTypeProp
{
    public override Type Prop
    {
        get => (Type)base.Prop;
    }
}

This will throw Microsoft.CSharp.RuntimeBinder.RuntimeBinderException: 'Property or indexer 'DerivedAttributeWithObjectProp.Prop' cannot be assigned to -- it is read only'

Replacing the dynamic with var will make it work.

Then if you replace the null with "Hello" (one of the examples that do work with custom attributes in my previous comment) it will no longer compile because now C# will require this to be a Type.

I think we're running into a bunch of undefined corner cases on the C# language side. What the reflection side does is a separate question, but I don't think we even know what property the blob is supposed to refer to when C# makes it, and C# might not be sure either.

[steveisok]

@pedrobsaila I appreciate the contribution. My read from Aaron's and Michal's comments is that this may take a while. I'm going to move this to draft. Please feel free to move to ready when the time comes.

[MichalStrehovsky]

I asked GPT-5.5 to explain what we're seeing. Roslyn has a very unique idea of what property the blob is setting, the algorithm is different from the default C# language overload resolution. Replicating it in reflection doesn't seem exactly trivial:

Copilot Chat Conversation Export: Roslyn attribute argument handling explained

User: @MichalStrehovsky
Thread URL: https://github.com/copilot/c/3fd3c235-0126-4ab8-a0ae-8e849d9e51ba

@MichalStrehovsky

using System;

var dd = new DerivedAttributeWithObjectProp();

// Below line doesn't compile (error CS0029: Cannot implicitly convert type 'string' to 'System.Type')
//dd.Prop = "Hello";

// Below line compiles just fine
[DerivedAttributeWithObjectProp(Prop = "Hello")]
public class BaseAttributeWithTypeProp : Attribute
{
    public virtual object Prop {  get; set; }
}

public class DerivedAttributeWithObjectProp : BaseAttributeWithTypeProp
{
    public override Type Prop => (Type)base.Prop;
}

Look at the two comments in this snippet. Why does one work and the other doesn't work. You have source code for the Roslyn C# compiler available here.

Copilot

The attribute form works because Roslyn gives named attribute arguments special handling for overridden properties: it walks back to the “least overridden” property and uses that property’s type/accessors for validation. In your example, that means it effectively validates Prop = "Hello" against the base object Prop { get; set; }, not against the overriding Type Prop => ....

The normal assignment does not use that special attribute rule. For dd.Prop = "Hello", the property visible on DerivedAttributeWithObjectProp has type System.Type, so Roslyn tries to convert "Hello" to System.Type and reports CS0029.

Key Roslyn code is in Binder_Attributes.cs:

// SPEC:    For each named-argument Arg in named-argument-list N:
// SPEC:        Let Name be the identifier of the named-argument Arg.
// SPEC:        Name must identify a non-static read-write public field or property on 
// SPEC:            attribute class T. If T has no such field or property, then a compile-time error occurs.

bool invalidNamedArgument = false;
TypeSymbol? namedArgumentType = null;
invalidNamedArgument |= (namedArgumentNameSymbol.DeclaredAccessibility != Accessibility.Public);
invalidNamedArgument |= namedArgumentNameSymbol.IsStatic;

if (!invalidNamedArgument)
{
    switch (namedArgumentNameSymbol.Kind)
    {
        case SymbolKind.Field:
            var fieldSymbol = (FieldSymbol)namedArgumentNameSymbol;
            namedArgumentType = fieldSymbol.Type;
            invalidNamedArgument |= fieldSymbol.IsReadOnly;
            invalidNamedArgument |= fieldSymbol.IsConst;
            break;

        case SymbolKind.Property:
            var propertySymbol = ((PropertySymbol)namedArgumentNameSymbol).GetLeastOverriddenProperty(this.ContainingType);
            namedArgumentType = propertySymbol.Type;
            invalidNamedArgument |= propertySymbol.IsReadOnly;
            var getMethod = propertySymbol.GetMethod;
            var setMethod = propertySymbol.SetMethod;
            invalidNamedArgument = invalidNamedArgument || (object)getMethod == null || (object)setMethod == null;
            if (!invalidNamedArgument)
            {
                invalidNamedArgument =
                    getMethod!.DeclaredAccessibility != Accessibility.Public ||
                    setMethod!.DeclaredAccessibility != Accessibility.Public;
            }
            break;

The important line is:

var propertySymbol = ((PropertySymbol)namedArgumentNameSymbol).GetLeastOverriddenProperty(this.ContainingType);
namedArgumentType = propertySymbol.Type;

GetLeastOverriddenProperty walks from an override down to the accessible base property:

internal PropertySymbol GetLeastOverriddenProperty(NamedTypeSymbol accessingTypeOpt)
{
    accessingTypeOpt = accessingTypeOpt?.OriginalDefinition;
    PropertySymbol p = this;
    while (p.IsOverride && !p.HidesBasePropertiesByName)
    {
        // We might not be able to access the overridden method. For example,
        // 
        //   .assembly A
        //   {
        //      InternalsVisibleTo("B")
        //      public class A { internal virtual int P { get; } }
        //   }
        // 
        //   .assembly B
        //   {
        //      InternalsVisibleTo("C")
        //      public class B : A { internal override int P { get; } }
        //   }
        // 
        //   .assembly C
        //   {
        //      public class C : B { ... new B().P ... }       // A.P is not accessible from here
        //   }
        //
        // See InternalsVisibleToAndStrongNameTests: IvtVirtualCall1, IvtVirtualCall2, IvtVirtual_ParamsAndDynamic.
        PropertySymbol overridden = p.OverriddenProperty;
        var discardedUseSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;
        if ((object)overridden == null ||
            (accessingTypeOpt is { } && !AccessCheck.IsSymbolAccessible(overridden, accessingTypeOpt, ref discardedUseSiteInfo)))
        {
            break;
        }

        p = overridden;
    }

    return p;
}

So for the attribute:

[DerivedAttributeWithObjectProp(Prop = "Hello")]

Roslyn:

1. Looks up Prop on DerivedAttributeWithObjectProp.
2. Sees it is an override.
3. Calls GetLeastOverriddenProperty(...).
4. Gets BaseAttributeWithTypeProp.Prop.
5. Uses its type: object.
6. Converts "Hello" to object, which is valid.

For the normal assignment:

dd.Prop = "Hello";

there is no attribute-specific “least overridden property” rule. The member access is bound as the overriding property whose type is System.Type, so the compiler checks whether string converts to System.Type. It does not, hence:

CS0029: Cannot implicitly convert type 'string' to 'System.Type'

I used GitHub code search while looking this up; search results are limited and may be incomplete. You can view more matches in GitHub UI here: https://github.com/search?q=repo%3Adotnet%2Froslyn+GetLeastOverriddenProperty+Binder_Attributes.cs&type=code

[dotnet-policy-service]

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.