I bet there's a pesky API guarantee that means this patch won't be acceptable. ;) The alternative is to just remove the 'const', I guess.


2008-03-28  Tim Retout  <tim@retout.co.uk>

        * pango/src/attributes.hg (gobj): Move const keyword to before the
        return value.  Fixes g++ 4.3 warning.

diff --git a/pango/src/attributes.hg b/pango/src/attributes.hg
index e8b374a..09d7a9f 100644
--- a/pango/src/attributes.hg
+++ b/pango/src/attributes.hg
@@ -115,7 +115,7 @@ public:
   /// Provides access to the underlying C GObject.  
   PangoAttribute*       gobj()       { return gobject_; }
   /// Provides access to the underlying C GObject.
-  PangoAttribute* const gobj() const { return gobject_; }
+  const PangoAttribute* gobj() const { return gobject_; }
 
   /** Create a new font family attribute.
    * @param family The family or comma separated list of families.

Could you check with nm to see if the symbols are different with this patch?

Well, I think the old one was not correct anyway.  The syntax was saying that it returned a constant pointer to a non-const PangoAttribute, when I think we wanted to return a pointer to a constant PangoAttribute (which is what the new version does).  

So this change has the potential to 'break' existing code that called gobj() on a const Pango::Attribute and then modified the pointed-to PangoAttribute.  But the intent of this API was to disallow that, and I don't imagine anybody intended to use it this way (since there's a non-const version as well).  So I think it's probably pretty safe to make the change.  Although if we really want to strictly guarantee API stability, we'd probably have to go with Tim's second option of removing the const altogether.

I had to compile with -O0 to get these symbols to appear in the output at all, but when I did, it appears the same with and without the patch:

$ nm -C pangomm/.libs/libpangomm-1.4.so.1.0.30 | grep Pango::Attribute::gobj
0001a864 W Pango::Attribute::gobj()
0001a86e W Pango::Attribute::gobj() const

Is there some other option I should be using, or is this sufficient?

(I can include ChangeLog entries actually as part of the diffs from now on - sorry, I didn't know that was the preference.)

ah, I suppose they're inlined when compiling with optimization since the implementation is in the header...  So there doesn't really seem to be any API compatibility issues in that case.

Yeah, I think it will be OK. Thanks for checking. Feel free to apply.

Created attachment 108322 [details] [review]
Patch with slightly better ChangeLog

Well, I don't have SVN commit access, if that was at me, but here's a patch including a better ChangeLog entry... there are too many consts involved.

2008-03-28  Tim Retout  <tim@retout.co.uk>

        * pango/src/attributes.hg (gobj): Return 'const PangoAttribute*'
        rather than 'PangoAttribute* const'.  Fixes g++ 4.3 warning.

Committed. Thanks.