[Mono-dev] another corcompare patch to hide extraneous "IsVirtual"differences

[Atsushi Eno]

Hi,

Kornél Pál wrote:
> Hi,
> 
> I understand your point of view why this case could be treated as OK but
> there is big difference between virtual and non-virtual methods. The most
> important and most relevant difference is that compilers (should) generate
> virtual calls for virtual methods and non-virtual calls for non-virtual
> methods even for sealed classes. If corcompare ignores this difference that
> will result in different code for calling the methods that is something 
> bad.
> In addition if these classes become non-sealed in later MS.NET versions
> (there are some such classes in MS.NET 2.0 that are non-sealed but were
> sealed in 1.x) or non-virtual methods become virtual the code will behave
> differently as virtual functions can be called using non-virtual IL code
> that results in the specified method instead of the virtual method that is
> something very bad.

Oh, good point. But that also sounds like those differences are
basically expected (between runtimes), thus it is not something we
must pay attension. Those people who depends on such tip of the
snow will be kicked by Microsoft when they compile their apps on
.NET 1.x and run them on .NET 2.0).

However, I agree with you that it could matter and had better
be fixed.

> So I think we should implement interface methods using the same syntax
> (using full name privately or using public methods with the same name) for
> full MS.NET compatibility.

Based on your points (and if I am correct), I think we had better warn
if those methods are non-virtual while MS ones are virtual. I have no
idea that virtcall could result in inconsistent call; we should anyways
keep warning on overridability (besides virtcall performance difference,
but I don't think it is something that should be warned in corcompare).

I don't think that internal interfaces (such as IConfigXmlNode I
mentioned before) should be warned as significant differences,so I
don't think warning every differences out is a good idea. (There are
also such classes that were added new interfaces in .NET 2.0 while
there weren't such in .NET 1.x.)

If this sounds reasonable, I'll bring another patch again.

Thanks,
Atsushi Eno

> 
> Kornél
> 
> ----- Original Message -----
> From: "Atsushi Eno" <atsushi at ximian.com>
> To: "mono-devel mailing list" <mono-devel-list at lists.ximian.com>
> Sent: Tuesday, November 01, 2005 11:07 AM
> Subject: [Mono-dev] another corcompare patch to hide extraneous
> "IsVirtual"differences
> 
> 
>> Hi,
>>
>> As implemented in the previous patch, corcompare now shows
>> abstract/virtual/static differnces on methods (including property
>> accessors).
>>
>> Now it uncovered the difference on how we implement interface
>> methods on each class. Say, there is only one Clear in our
>> System.CodeDom.NamespaceImportCollection.Clear(), while MS.NET
>> has two Clear() (one is an implicit implementation).
>>
>> One another case, there are some internal interfaces that
>> commonize some members (for example, ConfigXmlDocument that implements
>> IConfigXmlNode, in System.Configuration, has different IsVirtual
>> attribute on Filename property).
>>
>> Those differeces come from the fact that a MethodBase.IsVirtual is
>> True for such a method that is part of an interface implementation.
>>
>> They are not significant differences that we should care.
>>
>> Thus, the only differences we should care is whether they are
>> overridable or not. It prints "should (not) Overridable" instead of
>> "should (not) be Virtual".
>>
>> Based on the premise above, I created another corcompare patch.
>> This time it won't show such annoying differences.
>>
>> Does it sound fine?
>>
>> Atsushi Eno