451278 - OS/2 build break after commit 70c7932205ee due to gcc-3.3.5 parsing error

Status: RESOLVED WONTFIX

(Core :: XPCOM, defect)

Description

[Walter Meinl]

User-Agent:       Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.9.0.1) Gecko/2008071518 Firefox/3.0.1
Build Identifier: 

Checkin for bug265534 to reduce inlinings of string constructors broke OS/2 build, for the same compiler bug mentioned in bug345517c#46 the error output is pretty much similar to what was posted in bug345517c#35.
putting NS_COM at the end of the line in nsTSubstring.h and nsTString.h like it was done in https://bugzilla.mozilla.org/attachment.cgi?id=246540 lets the build go through xpcom. I'll attach a patch tomorrow as it is now too late to rebuild completely.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Peter Weilbacher]

Attachment: fix

It's pretty ugly with all those ifdefs but is there anything else we can do?

Comment 2

[Benjamin Smedberg]

Hrm, I am of the opinion that we should drop support for gcc < 3.4.

Comment 3

[Peter Weilbacher]

If we _had_ a newer GCC on OS/2 I would agree...

Comment 4

[Walter Meinl]

The patch works, though I'm wondering why
>+#ifdef XP_OS2 /* Workaround for GCC 3.3.x bug. */
>+      nsTSubstring_CharT() NS_COM;
>+#else
>       NS_COM nsTSubstring_CharT();
>+#endif
> 
>         // version of constructor that leaves mData and mLength uninitialized
>       explicit

>       NS_COM nsTSubstring_CharT( PRUint32 flags );

> 
>         // copy-constructor, constructs as dependent on given object
>         // (NOTE: this is for internal use only)
>+#ifdef XP_OS2 /* Workaround for GCC 3.3.x bug. */
>+      nsTSubstring_CharT( const self_type& str ) NS_COM;
>+#else
>       NS_COM nsTSubstring_CharT( const self_type& str );
>+#endif
it works though you didn't move NS_COM in the line I separated (I did it in my patch, worked as well, but even more cluttering!)

Comment 5

[Peter Weilbacher]

Walter, I think the problem is only with functions that are only prefixed with NS_COM. The one in the middle is decorated explicit already, that might explain the difference. At one point I read a GCC bug entry about this, but I haven't found it again.

Comment 6

[Benjamin Smedberg]

Comment on attachment 334654 [details] [diff] [review]
fix

Making the headers so much uglier like this is not an option.

Comment 7

[Dave Yeo]

in reply to Comment #5
The GCC bug entry is http://gcc.gnu.org/bugzilla/show_bug.cgi?id=6992

Comment 8

[Peter Weilbacher]

Benjamin, so what is your suggestion? Should I just move NS_COM to the end of the line for everybody? Or stop supporting OS/2?

Comment 9

[Benjamin Smedberg]

I do not think we can move NS_COM to the end of the line, because it would mis-place the dllexport annotation for MSVC. So, I think we should probably stop supporting OS/2 until they get a newer version of GCC.

Comment 10

[Peter Weilbacher]

Just to be clear about what our target needs to be: are you going to support GCC 3.4.x (and its bugs) through the whole of 1.9.x? Because if we start working on getting 3.4.6 now and it turns out in two months that another tiny change busts that, too, then we wasted a lot of time...

Btw, I fixed http://developer.mozilla.org/en/Linux_Build_Prerequisites to read GCC 3.4, too.

Comment 11

[Benjamin Smedberg]

We are going to support the compilers which take a reasonably small effort to support. I'm not going to make any promises about gcc 3.4.x in particular, though I don't know of any issues that would prevent them from working in the 1.9.x series

Comment 12

[Peter Weilbacher]

(In reply to comment #11)
> We are going to support the compilers which take a reasonably small effort to
> support.

That is a weird statement. After all, the effort to support GCC 3.3 _is_ already minimal for you. You (as in the reviewers) only need to glance at a tiny patch for like 30s and mark the bug r+. Us OS/2 people take care of the rest...

> I'm not going to make any promises about gcc 3.4.x in particular,
> though I don't know of any issues that would prevent them from working in the
> 1.9.x series

OK.

Comment 13

[Benjamin Smedberg]

I thought I made it clear why this patch was not acceptable: it adds ifdefs which make the code harder to read and maintain.

Comment 14

[Peter Weilbacher]

That's still a lame excuse as we OS/2ers usually (have to) do the maintenance of the stuff inside the XP_OS2 bits anyway and we are not duplicating many lines. But I probably wouldn't allow ugly bits in my code for platforms that I won't use, either. ;-)

Should I make a patch to revert the one XP_OS2 workaround that's already in there?

Btw, it seems that we are slowly getting somewhere with our efforts to get GCC 3.4.6 going on OS/2.

Comment 15

[Peter Weilbacher]

OK, Benjamin clearly said this is WONT for him and he doesn't seem to care about that stuff that's already in there. --> WONTFIX