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

[Kornél Pál]

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.

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.

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
>


--------------------------------------------------------------------------------


> Index: mono-api-diff.cs
> ===================================================================
> --- mono-api-diff.cs (revision 52436)
> +++ mono-api-diff.cs (working copy)
> @@ -1607,7 +1607,7 @@
>  {
>  None = 0,
>  Abstract = 1,
> - Virtual = 2,
> + Overridable = 2,
>  Static = 4
>  }
>
> @@ -1626,8 +1626,8 @@
>  flags |= SignatureFlags.Abstract;
>  if (((XmlElement) node).GetAttribute ("static") == "true")
>  flags |= SignatureFlags.Static;
> - if (((XmlElement) node).GetAttribute ("virtual") == "true")
> - flags |= SignatureFlags.Virtual;
> + if (((XmlElement) node).GetAttribute ("overridable") == "true")
> + flags |= SignatureFlags.Overridable;
>  if (flags != SignatureFlags.None) {
>  if (signatureFlags == null)
>  signatureFlags = new Hashtable ();
> Index: mono-api-info.cs
> ===================================================================
> --- mono-api-info.cs (revision 52436)
> +++ mono-api-info.cs (working copy)
> @@ -759,8 +759,8 @@
>
>  if (mbase.IsAbstract)
>  AddAttribute (p, "abstract", "true");
> - if (mbase.IsVirtual)
> - AddAttribute (p, "virtual", "true");
> + if (mbase.IsVirtual && !mbase.IsFinal)
> + AddAttribute (p, "overridable", "true");
>  if (mbase.IsStatic)
>  AddAttribute (p, "static", "true");
>
>


--------------------------------------------------------------------------------


> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>