[Mono-dev] Porting mcs to IKVM.Reflection

[Kornél Pál]

Hi,

Jeroen Frijters wrote:
> Kornél Pál wrote:
>> mcs creates an assembly, then creates types, and only after that is
>> processing the custom attributes. This is how attributes and even
>> assembly name (not the name part) can change after definition. Although
>> there are some other self referencing cases, this order of
>> creation/definition is required especially when building mscorlib.dll.
> 
> OK, that makes sense. I skipped custom attributes in my mcs IKVM.Reflection hack, so that may account for not running into that.
> 
> I don't really like AssemblyBuilder.__SetName(), however. How about adding __SetVersion(), __SetCulture() and __SetKeyPair()?

That would be a better idea. Updating Universe.assembliesByName also
would make sense.

>> Unlike Java, C# has support for backward jumps, so does
>> System.Reflection.Emit so support for that will have to be implemented.
> 
> The "backwards branch constraint" is an ECMA CIL restriction. The Microsoft CLR supports code that violates it, but years ago when I first found out about this it was because ikvm generated code that violated it and Mono didn't support that. So, I don't think it is necessary to support this (as it isn't valid ECMA CIL anyway). If mcs generates code that triggers this exception, then it is a bug in ILGenerator, or a bug in mcs.

Right, your check implements the ECMA requirements. There must be some
other problem then... I'll analyze that later.

>> Based on your questions I belive that you are trying to remain
>> compatible with MS.NET.
> 
> Yes. However, I'm all for adding extra functionality (either by __ methods or by having ___ settings to enable/disable specific behavior).

Using global (or instance-wide) flags may not be that good. Having
__methods that behave differently or have arguments (boolean or enum)
that either enable MS.NET compatible behavior or some other custom
behavior should be considered as well. In the latter case MS.NET
compatible methods could simply call the __ methods with parameters that
ensure MS.NET compatible behavior.

>> I belive that pseudo custom attributes and unmanaged resource generation
>> does not fit well to the concept of System.Reflection.Emit.
> 
> I kind of agree. I will add a mode to disable them and add specific __ methods to do the equivalent.

That would be awesome.

>> I also belive that CustomAttributeData should not be used as a provider,
>> __GetCustomAttributes (non-standard since is returning constructor
>> arguments rather than class instance) methods should be added to
>> reflection classes. An ICustomAttributeProvider interface (with
>> IsDefined and __GetCustomAttributes methods) would be very useful as
>> well.
> 
> Agreed.

Marek Safar wrote:
> I am not sure I follow here, CustomAttributeData exists because they
> behave very differently to ICustomAttributeProvider. Why would you unify
> them?

CustomAttributeData is not just the data store, it's methods are the
only public API to obtain custom attributes (using the provider design
pattern). ICustomAttributeProvider name can be retained because
IsDefined is totally compatible while using __GetCustomAttributes marks
that it is non-standard.

>>> I belive that pseudo custom attributes and unmanaged resource 
 >>> generation does not fit well to the concept of
>>> System.Reflection.Emit.
>>
>> I kind of agree. I will add a mode to disable them and add specific __
>> methods to do the equivalent.
>
> I am not that convinced, everyone writing non-trivial app will have to
> deal with them in some way.

My bad. I should have say that unmanaged resource generation does not 
well fit to the concept of AssemblyBuilder. There should be a better 
unmanaged resource generation class that fits System.Reflection.Emit 
better. The best example is that Mono's SRE has hack for unmanaged 
resource generation in compiler context mode.

After reading both of your responses I belive that pseudo custom 
attributes is not that harmful but there should be SetCustomAttribute 
methods (on each SRE class that supports custom attributes) that treats 
pseudo custom attributes like a compiler does, and a variant that just 
emits all the attributes.

Maybe there should be three modes:
- processing all pseudo custom attributes
- emitting all of them as custom attributes
- MS.NET compatible

Kornél