Hi, I think it would be useful to leave the old code for a while, better yet control it with an env var so we can tell people using packages to set the env var when debugging a bug.
Zoltan On Feb 4, 2008 5:39 PM, Massimiliano Mantione <[EMAIL PROTECTED]> wrote: > > On Mon, 2008-02-04 at 15:22 +0100, Paolo Molaro wrote: > > Did you run the corlib tests, too? > > I run "make check" in mono, which runs just about everything, > also after a full rebuild (done with the new code). > > > > + // Check if this interface is explicitly implemented (instead > > > of just inherited) > > > + if (parent != NULL) { > > > + int implemented_interfaces_index; > > > + interface_is_explicitly_implemented_by_class = FALSE; > > > + for (implemented_interfaces_index = 0; > > > implemented_interfaces_index < class->interface_count; > > > implemented_interfaces_index++) { > > > + if (ic == class->interfaces > > > [implemented_interfaces_index]) { > > > + > > > interface_is_explicitly_implemented_by_class = TRUE; > > > + break; > > > + } > > > + } > > > > You likely need to loop other all the hierarchy here, right? Because > > this is not restricted to just the immediate parent. > > Well, in section 2, 12.2, the standard says "If this class explicitly > specifies that it implements the interface (i.e., the interfaces that > appear in this class's InterfaceImpl table, ยง22.23)". > In the code, "interface_is_explicitly_implemented_by_class" wants to > flag this condition, so I only look in "class->interfaces". > > > > + /* Why do we need this? */ > > > + if (vtable [im_slot]->slot < > > > 0) { > > > + vtable > > > [im_slot]->slot = im_slot; > > > + } > > > > It's much simpler to use: > > if (cm->slot < 0) > > cm->slot = im_slot; > > Yes, right :-) > > > > + // If the slot is still empty, look in all > > > the inherited virtual methods... > > > + if ((vtable [im_slot] == NULL) && > > > class->parent != NULL) { > > > + MonoClass *parent = class->parent; > > > > I think you need to loop over all the parents here. Please write the > > appropriate test cases where the interface method is implemented by > > a non-immediate parent. > > > > > + // Reverse order, so that last added > > > methods are preferred > > > + for (cm_index = parent->vtable_size - > > > 1; cm_index >= 0; cm_index--) { > > > + MonoMethod *cm = > > > parent->vtable [cm_index]; > > Always in section 2, 12.2, the standard says "If there are any virtual > methods in the interface that still have empty slots, see if there are > any public virtual methods, but not public virtual newslot methods, > available on this class (directly or inherited)...". > My interpretation is that since we are looking for virtual methods they > must be in the vtable, and "parent->vtable" contains all of them (also > the inherited ones), so one single loop is OK. > The methods of "class" are already covered by the previous loop. > > Just to be on the safe side, I modified iface4.cs to test this as > well (maybe there was already a test like that somewhere, but it was > easier adding it anyway), and it passes. > > > > + if (vtable [im_slot] == NULL) { > > > + int index; > > > + char *method_signature; > > > + for (index = 0; index < onum; > > > ++index) { > > > + g_print (" at slot %d: %s > > > (%d) overrides %s (%d)\n", im_slot, overrides [index*2+1]->name, > > > + overrides > > > [index*2+1]->slot, overrides [index*2]->name, overrides [index*2]->slot); > > > + } > > > + method_signature = > > > mono_signature_get_desc (mono_method_signature (im), FALSE); > > > + printf ("no implementation for > > > interface method %s::%s(%s) in class %s.%s\n", > > > + mono_type_get_name > > > (&ic->byval_arg), im->name, method_signature, class->name_space, > > > class->name); > > > + g_free (method_signature); > > > + for (index = 0; index < > > > class->method.count; ++index) { > > > + MonoMethod *cm = > > > class->methods [index]; > > > + method_signature = > > > mono_signature_get_desc (mono_method_signature (cm), TRUE); > > > + > > > + printf ("METHOD %s(%s)\n", > > > cm->name, method_signature); > > > + g_free (method_signature); > > > + } > > > > Move this debugging code to it's own external function so it doesn't > > clutter the code. > > OK. > > The last question: should I leave > > #define USE_NEW_INTERFACE_VTABLE_CODE 1 > > in place (with the old code) for a few days after committing (just in > case, for debugging), or should I commit it directly clean? > > Thanks a lot! > Massi > > > > _______________________________________________ > Mono-devel-list mailing list > Mono-devel-list@lists.ximian.com > http://lists.ximian.com/mailman/listinfo/mono-devel-list > _______________________________________________ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list