622258 - AIX 5.1 backport: compilation error in widget/src/gtk2/nsWindow.cpp

Status: RESOLVED WORKSFORME

(Core :: Widget: Gtk, defect)

Description

[Uli Link (:ul-mcamafia)]

When compiling Firefox 3.6.13 on AIX 5.1 with IBM XLC/C++ 8.0 I get the following compilation error:

SSIFIER=1 -DMOZ_LOGGING=1 -DMOZ_USER_DIR=\".mozilla\" -DMOZ_ENABLE_LIBXUL=1 -DHAVE_STDINT_H=1 -DHAVE_INTTYPES_H=1 -DMOZ_XUL=1 -DMOZ_PROFILELOCKING=1 -DMOZ_RDF=1 -DBUILD_CTYPES=1 -DMOZ_MORKREADER=1 -DMOZ_DLL_SUFFIX=\".so\" -DHAVE_FONTCONFIG_FCFREETYPE_H=1 -DXP_UNIX=1 -DUNIX_ASYNC_DNS=1 -DMOZ_ACCESSIBILITY_ATK=1 -DATK_MAJOR_VERSION=1 -DATK_MINOR_VERSION=12 -DATK_REV_VERSION=3  -D_MOZILLA_CONFIG_H_ -DMOZILLA_CLIENT /home/ulink/src/mozilla-1.9.2/widget/src/gtk2/nsWindow.cpp
"../../../dist/include/nsAutoRef.h", line 648.12: 1540-1196 (S) The return type cannot be "pixman_region32" because "struct pixman_region32" does not have an "operator->" function.
"../../../dist/include/nsAutoRef.h", line 139.26: 1540-0700 (I) The previous message was produced while processing "class nsAutoRefBase<pixman_region32>".
"../../../dist/include/nsAutoRef.h", line 139.7: 1540-0700 (I) The previous message was produced while processing "class nsAutoRef<pixman_region32>".
"/home/ulink/src/mozilla-1.9.2/widget/src/gtk2/nsWindow.cpp", line 1905.1: 1540-0700 (I) The previous message was produced while processing "nsWindow::Scroll(const nsIntPoint &, const nsTArray<nsIntRect> &, const nsTArray<nsIWidget::Configuration> &)".
gmake[1]: *** [nsWindow.o] Error 1
gmake[1]: Leaving directory `/home/ulink/src/mozilla-1.9.2/obj-fx36-aix51/widget/src/gtk2'
gmake: *** [default] Error 2
bash-3.2$ 

The compilation error does not happen with IBM XLC/C++ 9.0 or newer, but XLC9 is not available for AIX 5.1 which means dropping older hardware platforms.

Don't know where to insert the missing "operator ->", but fix should be easy and portable.

Comment 1

[Uli Link (:ul-mcamafia)]

Seems the affected code was introduced with the Lorenz merge.

Comment 2

[Karl Tomlinson (:karlt)]

pixman_region32 is a C struct, for which operator-> doesn't make sense.

In template <class T> nsAutoRefBase,

    RawRef operator->() const
    {
        return this->get();
    }

is only expected to be used for some T, but not when T is a POD struct (pixman_region32).  Apparently XLC/C++ 8.0 is trying to define this for nsAutoRefBase<pixman_region32>, even though, I assume, it is not used.
I don't know whether there is a way to persuade the compiler not to define this function.  You may like to try commenting out this method in the template declaration, but there may be other consumers that need it.

Comment 3

[Uli Link (:ul-mcamafia)]

After commenting out the |operator->()| in the nsAutoRef template as suggested, nsWindow.cpp compiled fine. But (as expected...) in gfx/thebes/src/gfxPangoFonts.cpp an related error then occurs:

...\" -DHAVE_FONTCONFIG_FCFREETYPE_H=1 -DXP_UNIX=1 -DUNIX_ASYNC_DNS=1 -DMOZ_ACCESSIBILITY_ATK=1 -DATK_MAJOR_VERSION=1 -DATK_MINOR_VERSION=12 -DATK_REV_VERSION=3  -D_MOZILLA_CONFIG_H_ -DMOZILLA_CLIENT /home/ulink/src/mozilla-1.9.2/gfx/thebes/src/gfxPangoFonts.cpp
"/home/ulink/src/mozilla-1.9.2/gfx/thebes/src/gfxPangoFonts.cpp", line 1308.36: 1540-0202 (S) An expression of type "nsAutoRef<_FcFontSet>" is not allowed on the left side of "->".
"/home/ulink/src/mozilla-1.9.2/gfx/thebes/src/gfxPangoFonts.cpp", line 1309.38: 1540-0202 (S) An expression of type "nsAutoRef<_FcFontSet>" is not allowed on the left side of "->".
"/home/ulink/src/mozilla-1.9.2/gfx/thebes/src/gfxPangoFonts.cpp", line 1355.44: 1540-0202 (S) An expression of type "nsAutoRef<_FcFontSet>" is not allowed on the left side of "->".
"/home/ulink/src/mozilla-1.9.2/gfx/thebes/src/gfxPangoFonts.cpp", line 1356.41: 1540-0202 (S) An expression of type "nsAutoRef<_FcFontSet>" is not allowed on the left side of "->".
"/home/ulink/src/mozilla-1.9.2/gfx/thebes/src/gfxPangoFonts.cpp", line 1389.42: 1540-0202 (S) An expression of type "nsAutoRef<_FcFontSet>" is not allowed on the left side of "->".
gmake[5]: *** [gfxPangoFonts.o] Error 1

So the operator seems to exist intentionally. 
The solution should depend on the type of the template parameter. if T is a class then implement |operator->| , if T is of POD type struct then not implement it.

karlt: thx for your help, hard stuff for me, as I'm not a C++ guru at all.

Comment 4

[Karl Tomlinson (:karlt)]

(In reply to comment #3)
> The solution should depend on the type of the template parameter. if T is a
> class then implement |operator->| , if T is of POD type struct then not
> implement it.

Yes.  I'm not certain about what the standard requires but my expectation was
that such inline methods of a class template would only be implemented for
particular T if used with that T.  It sounds like XLC/C++ 8.0 is implementing
the whole class rather than individual methods, if any method in the class is
used.

I'm only making a guess but perhaps this compiler might behave differently if
the definition of operator-> is separate from the class declaration (but still
in the same header file), as for Foo<T>::someMethod(T x) here:
http://www.parashift.com/c++-faq-lite/templates.html#faq-35.12

If that fails, then a workaround could be to surround operator-> by a
preprocessor conditional such as "ifndef NS_AUTOREF_SUPPRESS_MEMBER_POINTER",
and to define this in nsWindow before including nsAutoRef.h.

Comment 5

[Uli Link (:ul-mcamafia)]

Attachment: ifdef'd template specialization without the useless operator

Took the template from xpcom/base/nsAutoRef.h and copied it into nsWindow.cpp and altered to a specialization of type pixman_region32 and removed the failing |operator->()|. 
Conditionally compiling only on AIX and if IBM XLC/C++ is older than version 9.
A lot of code, but minimal impact. 
Was my very first template specialization ;-) I'm sure there is potential for further optimization.

Comment 6

[Karl Tomlinson (:karlt)]

Comment on attachment 501325 [details] [diff] [review]
ifdef'd template specialization without the useless operator

Well done, Uli.  This looks like a correct solution.

I see on wikipedia that 5.1 support was discontinued April 2006.

I'm pleased you've found a solution, but given that and also the discontinued
support from compiler vendors, I doubt Mozilla wants to maintain workarounds
for AIX 5.1.  If there's a good reason to support this platform then I guess
we could consider this for 1.9.2, as this was a branch regression for that
platform.

Comment 7

[Uli Link (:ul-mcamafia)]

(In reply to comment #6)

> I see on wikipedia that 5.1 support was discontinued April 2006.

That's true for the Operating system. The used IBM XLC/C++ 8.0 compiler's EoL is mid 2011. Building with XLC8 needs also --disable-accessibility as described in https://bugzilla.mozilla.org/show_bug.cgi?id=501924#c5
 
> I'm pleased you've found a solution, but given that and also the discontinued
> support from compiler vendors, I doubt Mozilla wants to maintain workarounds
> for AIX 5.1.  If there's a good reason to support this platform then I guess
> we could consider this for 1.9.2, as this was a branch regression for that
> platform.

There are a few reasons why I try to build on AIX 5.1:
- AIX 5.1 was the last to support the original RS/6000 Microchannel and PReP machines (MCA was MicroChannelArchticture, that's were my nick comes from...)
- AIX has full binary compatibility

I think it's ok, not to take this patch on branch, and use it locally for my AIX 5.1 builds instead. Far too much additional lines for a tier x+n legacy platform.
For the 1.9.1 branch I have all but one trivial patches for AIX 5.1 in the repository, and for my backport to even older AIX 4.3.3 I have a few porting hunks locally. On AIX 5.3 Firefox 3.5.11 and later builds fine without any patches.

Most of the patches needed for building with IBM XLC/C++ 9 on AIX 5.3 are r+ and I would like to have those on the branch, so mozilla-1.9.2 builds out of the box just like the 1.9.1 branch.

Now I know what is the minimum compiler version needed for checking in toplevel configure.in, and I last changed that item for 1.8.1 branch.

karlt: many THX for your help.