GNOME Bugzilla – Bug 745068
[Patch] Make gtype macros const compliant
Last modified: 2018-05-24 17:30:45 UTC
The recent G_DECLARE_* macros replace the old #define boilerplate. This is cool, but introduce a small "regression". Before, passing a const pointer to FOO_IS_A_BAR was fine. Now that it's a function, it prints a lot of warnings and I'm not convinced casting as non-const is a proper solution here. Change these functions to take const parameters instead. Btw, do the same for *_get_instance_private
Created attachment 297736 [details] [review] the G_DECLARE_* part
Created attachment 297737 [details] [review] the *_get_instance_private part
Review of attachment 297737 [details] [review]: To be perfectly honest, and a bit blunt, nobody really writes getters with a const GObject instance in the G* platform.
I get that, but is there any drawback in supporting it?
I don't see any drawback here...
Review of attachment 297736 [details] [review]: Looks fine (although I do question the style of any code that would depend on these changes).
Review of attachment 297737 [details] [review]: Also fine.
Review of attachment 297736 [details] [review]: commited as 52f23db74ad58b822bafb0fdcac106489d864f8c
Review of attachment 297737 [details] [review]: commited as a3a9664ed202303b899ca55625877542309d1a1f
The drawback is that now G++ and GCC with -Wcast-qual warn because of the mismatched constness between the inline functions and the underlying GType API, which does not use const pointers. See bug: 748631. Either we revert this and we don't allow passing const pointers — which, to me, is perfectly acceptable; const GObject instances barely make sense — or we constify the GType API, which will likely generate more warnings.
*** Bug 748631 has been marked as a duplicate of this bug. ***
I've pushed the reverts to GLib master. If we want to constify arguments we'll have to figure some other way to do it, and ensure that C++ compilers won't break. I'll leave the bug open in the meantime.
I don't think reverting the patches was the best way to solve the issue. Now, we're not able to make self-explanatory, clean API using Glib. The problem I'm hitting right now, is that I can't compile following code without warnings: void method(const SomeType *my_instance) { G_TYPE_CHECK_INSTANCE_TYPE (my_instance, some_type_get_type()); // Warning: discard const qualifier } So now I have to either explicitly cast my_instance to (SomeType*), what is very inconvenient, or remove "const" from the method's declaration, what is unacceptable(it makes my API non-self-explanatory). Maybe we should rather spend more time on fixing GType API, and systematically fix the warnings. If you agree, I can work on it as a volunteer and provide some patches.
(In reply to Marcin Kolny (IRC: loganek) from comment #13) > I don't think reverting the patches was the best way to solve the issue. > Now, we're not able to make self-explanatory, clean API using Glib. The patches broke perfectly valid code, by introducing warnings all over the place. Since you're the only person that opened a bug to introduce constness annotations to the macros, reverting the patches was the obvious choice. > The problem I'm hitting right now, is that I can't compile following code > without warnings: > > void method(const SomeType *my_instance) > { > G_TYPE_CHECK_INSTANCE_TYPE (my_instance, some_type_get_type()); // > Warning: discard const qualifier > } This code is not common in any project using GObject. Accessors that do not modify the instance are rare, and they usually come with side effects anyway. What happens if the accessor does not modify the instance state but still has to emit a signal? Or notify a read-only property? Anything that may take a temporary reference within the method will need a non-const pointer, even if the instance itself won't be modified. > So now I have to either explicitly cast my_instance to (SomeType*), what is > very inconvenient, or remove "const" from the method's declaration, what is > unacceptable(it makes my API non-self-explanatory). > > Maybe we should rather spend more time on fixing GType API, and > systematically fix the warnings. Or maybe you should *not* use `const`, like everybody else. > If you agree, I can work on it as a volunteer and provide some patches. I don't particularly agree; I think it's not worth the aggravation to handle `const` at the C level on GObject. I am not the maintainer of GLib, though, so if Allison agrees to have every function in GTypeInstance take a const then you can start working on it. I'd like to point out that adding const annotations to the GTypeInstance API may very well end up generating more warnings.
*** Bug 772430 has been marked as a duplicate of this bug. ***
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/997.