Re: warning about invalid tree model iterators

[Tim Janik <timj imendio com>]

On Tue, 21 Mar 2006, Yevgen Muntyan wrote:

> Tim Janik wrote:

> > to help the compiler catch these mistakes, i've prepared a patch that adds
> > G_GNUC_WARN_UNUSED_RESULT to all relevant iterator functions, and intend
> > to commit that next week unless objections pop up. in principle it does:
>
> These warnings flags generate too many false warnings, i.e. warnings in cases
> where it's perfectly okay to ignore return value.
> Examples of such uses are the following:
> 1) When you know exactly what your model contains, e.g. when you know it's 
> not
> empty, and you are getting iter pointing to the first node. Or, when you get 
> number
> of nodes in the model, and then iterating over them in a for loop.
> 2) When you get path/iterator somehow, with error checking, and then convert 
> it
> to iterator/path. E.g. you can get a path from a row reference, and if it's 
> not NULL,
> you know it's a valid path, so you don't need to check return value of
> gtk_tree_model_get_iter.

thanks for pointing this out.
first, i have to admit that i based my initial decision on the fact that adding
G_GNUC_WARN_UNUSED_RESULT to the tree API generates *0* warnings with CVS gtk+.

and - that is indeed the case if you compile with gcc-3.3 ;)

gcc-3.3 simply has no support for the flag, and i erroneously used it to
benchmark the effect of flagging the tree view API. compiling with gcc-3.4
does show the loads of warnings you describe.

> These two cases pretty much cover all the methods with added warnings (watch
> gtk compilation for number of examples of wrong warnings), so while these 
> warnings
> may help to catch bugs, they also help to add new bugs, since one either 
> starts
> ignoring warnings or is forced to suppress warnings using dummy
> variables.

valid point. i was willing to accept a *few* bogus warnings, but introducing
a major inconvenience in the tree view API is not useful.

> Therefore, either these warnings flags should be removed, or there should be
> introduced something that allows one to disable them (or rather enable).
> I believe the real solution would be using some tool like splint which would
> allow to suppress/force warnings in given places. GCC attribute doesn't
> seem to be intelligent enough to handle GtkTreeModel api ;)
>
> Note that disabling G_GNUC_WARN_UNUSED_RESULT globally would not be
> a nice solution, since it's usually good to have unused-result warnings from
> GList api, for example; and it's much less common to ignore return value
> of, say, g_list_delete_link (though, of course, it's possible to write code 
> like
> list = g_list_append(NULL, NULL); g_list_delete_link(list, list);).

disabling G_GNUC_WARN_UNUSED_RESULT globally is not what anyone of us had
in mind i think. i agree with you that the patch should simply be backed
out where it generates too many warnings.

did you, or anyone else for that matter, investigate yet whether it'd be worth
keeping the warnings for *some* of the functions?

> Best regards,
> Yevgen

---
ciaoTJ