q66 pushed a commit to branch master. http://git.enlightenment.org/core/efl.git/commit/?id=d9dbf886211ecbdcfb70c5021070dd1f93636c87
commit d9dbf886211ecbdcfb70c5021070dd1f93636c87 Author: Daniel Kolesa <[email protected]> Date: Wed Jan 23 17:59:40 2019 +0100 eolian: get rid of false positives about unimplemented methods We worked under the assumption that when inheriting callables from a regular class, it's good enough to just set those as fully implemented, because the underlying class is already checked. This assumption is wrong, as the callables may contain multiple implements pointing at the same function (consider when a regular class implements just the get part of a property but some underlying class implements it whole) - the old logic would result in just the first reached implement (possibly incomplete) being added into callables of the inheriting class, which results in false positives. Consider this example: class A has a fully implemented property foo class B inherits from A and partially implements foo abstract C inherits from B class D inherits from C abstract C would go over B's implements, encounter the first partial implementation of foo, adding it into its own callables and marking it as fully implemented (because it came from an already checked regular class B), which would result in the other implement being skipped. So make no assumptions and use the same logic for all class types. Of course, this brings in another problem: some errors would now get printed twice, because if you have a class A which has funcs that are unimplemented and class B inheriting from it, errors would get printed for A but also for B, which would include A's errors. To battle that, introduce a "global" (well, local to the entry point of the validator) hash tracking which implements have already been errored on; and skip those appropriately. --- src/lib/eolian/database_validate.c | 46 ++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/src/lib/eolian/database_validate.c b/src/lib/eolian/database_validate.c index b9f675b4ed..d2b2f8fa04 100644 --- a/src/lib/eolian/database_validate.c +++ b/src/lib/eolian/database_validate.c @@ -713,17 +713,16 @@ _db_fill_callables(Eolian_Class *cl, Eolian_Class *icl, Eina_Hash *fs, Eina_Bool Eina_Bool allow_impl = parent || (icl->type == EOLIAN_CLASS_MIXIN); EINA_LIST_FOREACH(icl->callables, l, impl) { - Impl_Status ost = (Impl_Status)eina_hash_find(fs, &impl->foo_id); - Eina_Bool extd = (ost != IMPL_STATUS_FULL); - if (icl->type == EOLIAN_CLASS_REGULAR) - /* stuff coming from full classes is assumed to be already checked - * so we are sure that everything is implemented or composite'd - */ - eina_hash_set(fs, &impl->foo_id, (void *)IMPL_STATUS_FULL); - else - extd = _extend_impl(fs, impl, !allow_impl); - if (extd) + /* while regular classes are already fully checked and one may + * assume that we could just make everything coming from regular + * classes IMPL_STATUS_FULL, we still need to account for all of + * the callables of the regular class, as the full implementation + * may come from somewhere deeper in the inheritance tree and we + * may not reach it first, so follow the same logic for all + */ + if (_extend_impl(fs, impl, !allow_impl)) { + Impl_Status ost = (Impl_Status)eina_hash_find(fs, &impl->foo_id); /* we had an unimplementation in the list, replace * instead of appending the new thing to callables * this is a corner case, it shouldn't happen much @@ -745,7 +744,8 @@ _db_fill_callables(Eolian_Class *cl, Eolian_Class *icl, Eina_Hash *fs, Eina_Bool } static Eina_Bool -_db_check_implemented(Validate_State *vals, Eolian_Class *cl, Eina_Hash *fs, Eina_Hash *cs) +_db_check_implemented(Validate_State *vals, Eolian_Class *cl, Eina_Hash *fs, + Eina_Hash *cs, Eina_Hash *errh) { if (cl->type != EOLIAN_CLASS_REGULAR) return EINA_TRUE; @@ -767,6 +767,11 @@ _db_check_implemented(Validate_State *vals, Eolian_Class *cl, Eina_Hash *fs, Ein */ if (eina_hash_find(cs, &fid->klass)) continue; + /* the error on this impl has already happened, which means it came + * from another regular class; reduce verbosity by not repeating it + */ + if (eina_hash_find(errh, &impl)) + continue; switch (st) { case IMPL_STATUS_NONE: @@ -774,6 +779,7 @@ _db_check_implemented(Validate_State *vals, Eolian_Class *cl, Eina_Hash *fs, Ein &cl->base, "unimplemented function '%s' (originally defined at %s:%d:%d)", fid->base.name, fid->base.file, fid->base.line, fid->base.column); succ = EINA_FALSE; + eina_hash_set(errh, &impl, impl); continue; case IMPL_STATUS_GET: case IMPL_STATUS_SET: @@ -781,6 +787,7 @@ _db_check_implemented(Validate_State *vals, Eolian_Class *cl, Eina_Hash *fs, Ein &cl->base, "partially implemented function '%s' (originally defined at %s:%d:%d)", fid->base.name, fid->base.file, fid->base.line, fid->base.column); succ = EINA_FALSE; + eina_hash_set(errh, &impl, impl); continue; case IMPL_STATUS_FULL: continue; @@ -942,7 +949,8 @@ _add_composite(Eolian_Class *cl, const Eolian_Class *icl, Eina_Hash *ch) } static Eina_Bool -_db_fill_inherits(Validate_State *vals, Eolian_Class *cl, Eina_Hash *fhash) +_db_fill_inherits(Validate_State *vals, Eolian_Class *cl, Eina_Hash *fhash, + Eina_Hash *errh) { if (eina_hash_find(fhash, &cl->base.name)) return EINA_TRUE; @@ -966,7 +974,7 @@ _db_fill_inherits(Validate_State *vals, Eolian_Class *cl, Eina_Hash *fhash) * the rest of the list needs to be freed in order not to * leak any memory */ - succ = _db_fill_inherits(vals, cl->parent, fhash); + succ = _db_fill_inherits(vals, cl->parent, fhash, errh); } } @@ -977,7 +985,7 @@ _db_fill_inherits(Validate_State *vals, Eolian_Class *cl, Eina_Hash *fhash) if (!succ) continue; cl->extends = eina_list_append(cl->extends, out_cl); - succ = _db_fill_inherits(vals, out_cl, fhash); + succ = _db_fill_inherits(vals, out_cl, fhash, errh); } if (succ && cl->type == EOLIAN_CLASS_MIXIN) @@ -993,7 +1001,7 @@ _db_fill_inherits(Validate_State *vals, Eolian_Class *cl, Eina_Hash *fhash) } if (succ) { - _db_fill_inherits(vals, out_cl, fhash); + _db_fill_inherits(vals, out_cl, fhash, errh); } if (!succ) continue; @@ -1070,7 +1078,7 @@ _db_fill_inherits(Validate_State *vals, Eolian_Class *cl, Eina_Hash *fhash) _db_fill_callables(cl, icl, fh, EINA_FALSE); /* verify that all methods are implemented on the class */ - if (!_db_check_implemented(vals, cl, fh, ch)) + if (!_db_check_implemented(vals, cl, fh, ch, errh)) vals->warned = EINA_TRUE; eina_hash_free(fh); @@ -1297,14 +1305,18 @@ database_validate(const Eolian_Unit *src) /* do an initial pass to refill inherits */ Eina_Iterator *iter = eolian_unit_classes_get(src); Eina_Hash *fhash = eina_hash_pointer_new(NULL); + /* keeps track of impls we already errored on to reduce verbosity */ + Eina_Hash *errh = eina_hash_pointer_new(NULL); EINA_ITERATOR_FOREACH(iter, cl) { - if (!_db_fill_inherits(&vals, cl, fhash)) + if (!_db_fill_inherits(&vals, cl, fhash, errh)) { + eina_hash_free(errh); eina_hash_free(fhash); return EINA_FALSE; } } + eina_hash_free(errh); eina_hash_free(fhash); eina_iterator_free(iter); --
