[Mono-dev] Bugzilla Bug 475860, or, "the wrong patch for the right job"

[Marek Safar]

Hi,
> I'm investigating http://bugzilla.novell.com/show_bug.cgi?id=475860; I've
> come up with a patch that addresses the issue, but I'm certain that it's the
> wrong way to solve the problem. Here's the patch:
>
> ---
> Index: ecore.cs
> ===================================================================
> --- ecore.cs	(revision 126868)
> +++ ecore.cs	(working copy)
> @@ -4027,6 +4027,12 @@
>  						Methods [j++] = m;
>  				}
>  				nmethods = j;
> +
> +				if (nmethods == 0 && candidate_overrides != null) {
> +					foreach (MethodBase m in candidate_overrides)
> +						Methods [j++] = m;
> +					nmethods = j;
> +				}
>  			}
>  
>  			//
> @@ -4291,11 +4297,11 @@
>  			// If the method is a virtual function, pick an override closer to the
> LHS type.
>  			//
>  			if (!IsBase && best_candidate.IsVirtual) {
> -				if (TypeManager.IsOverride (best_candidate))
> +/*				if (TypeManager.IsOverride (best_candidate))
>  					throw new InternalErrorException (
> -						"Should not happen.  An 'override' method took part in overload
> resolution: " + best_candidate);
> +						"Should not happen.  An 'override' method took part in overload
> resolution: " + best_candidate);*/
>  
> -				if (candidate_overrides != null) {
> +				if (!TypeManager.IsOverride (best_candidate) && candidate_overrides !=
> null) {
>  					Type[] gen_args = null;
>  					bool gen_override = false;
>  					if (TypeManager.IsGenericMethod (best_candidate))
> ---
>
> The reason that the code listed in the Bugzilla bug fails to compile is that
> at the time of overload resolution, the method table for the family of
> Reference classes has not been completely initialized. Because of this, when
> TypeManager.TryGetBaseDefinition is called, it fails, and no candidates are
> added to the list of method overloads.
>
> The patch included above addresses this problem by allowing overrides to be
> added to the list of candidates if no other candidates have been found.
> While this allows the example code to compile and even produces correct
> results at runtime, I'm pretty sure that the most appropriate way to solve
> this problem would be to instead make sure that the method table is
> appropriately populated before we even get to this code (also, a comparison
> of the IL produced by gmcs and the IL produced by csc reveals that csc
> produces virtual calls to Reference::CompareTo(), while gmcs produces
> virtual calls to ReferencePoint::CompareTo() and
> ReferenceRange::CompareTo()).
>
> I'm afraid that fixing this bug will require a deeper knowledge of the
> compiler than I currently possess. Can anyone suggest a more appropriate way
> of solving this problem?
>   
The right fix is to change how MemberCache is populated in such 
scenarios, we have several similar bugs reported and they are all 
related to missing members/types in type's MemberCache.

Marek