After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 752469 - Fix the build with -Wshadow
Fix the build with -Wshadow
Status: RESOLVED FIXED
Product: gtkmm
Classification: Bindings
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkmm-forge
gtkmm-forge
Depends on:
Blocks:
 
 
Reported: 2015-07-16 07:36 UTC by Murray Cumming
Modified: 2015-08-12 19:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
0001-Fix-the-build-with-Wshadow-compiler-warnings.patch (25.09 KB, patch)
2015-07-16 07:37 UTC, Murray Cumming
committed Details | Review

Description Murray Cumming 2015-07-16 07:36:29 UTC
This patch adds -Wshadow to the --enable-warnings=fatal build. It doesn't catch any real errors in gtkmm yet, but I like how this warning can catch real errors in future, as it has in some of my application code. And, by renaming some of gtkmm's parameter names, it helps application developers to avoid -Wshadow warnings at first when they override our virtual methods.

However, renaming method parameters such as "GdkEvent* event" means that the generated documentation for that parameter no longer matches. And writing out all the method's documentation by hand would be tedious. So this needs work.
Comment 1 Murray Cumming 2015-07-16 07:37:08 UTC
Created attachment 307531 [details] [review]
0001-Fix-the-build-with-Wshadow-compiler-warnings.patch
Comment 2 Kjell Ahlstedt 2015-07-16 14:07:52 UTC
We've got the gmmproc feature cpp_name{c_name} which makes it possible to change
the order of parameters, but which can also change the parameter names,
for example
  _WRAP_METHOD(int event(const Glib::RefPtr<CellAreaContext>& context,
    Widget& widget, GdkEvent* gdk_event{event}, const Gdk::Rectangle& cell_area,
    GtkCellRendererState  flags), gtk_cell_area_event)

Unfortunately, it does not change the parameter name in the documentation,
but that can probably be fixed in gmmproc.
Comment 3 Kjell Ahlstedt 2015-07-23 15:17:02 UTC
I have pushed the glibmm patch
https://git.gnome.org/browse/glibmm/commit/?id=1bed47055676dbc0bdfc5f2542781770bbe4797c

It changes parameter names in documentation to the names in _WRAP_METHOD() or
_WRAP_SIGNAL() even without {c_name} or {.}. I haven't tested it with the patch
in comment 1, but quite a few parameter names already differ between C code and
C++ code in glib/glibmm and gtk+/gtkmm, so lots of test cases are available
without that patch.

Gmmproc can now also more accurately distinguish between non-static and static
methods when it generates documentation, i.e. methods where documentation of
the first C parameter shall or shall not be removed.
Comment 4 Kjell Ahlstedt 2015-08-12 18:43:31 UTC
The -Wshadow patch does not cause mismatching parameter names between
documentation and function declarations, if the latest gmmproc is used.
Comment 5 Murray Cumming 2015-08-12 19:25:05 UTC
Thanks. Pushed.