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

RESOLVED WONTFIX

Status

()

Core
String
--
blocker
RESOLVED WONTFIX
10 years ago
10 years ago

People

(Reporter: Walter Meinl, Assigned: Peter Weilbacher)

Tracking

Trunk
x86
OS/2
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

fix
3.95 KB, patch
Benjamin Smedberg
: review-
Details | Diff | Splinter Review
(Reporter)

Description

10 years ago
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.
(Assignee)

Updated

10 years ago
Blocks: 265534
Severity: normal → blocker
Version: unspecified → Trunk
(Assignee)

Comment 1

10 years ago
Created attachment 334654 [details] [diff] [review]
fix

It's pretty ugly with all those ifdefs but is there anything else we can do?
Assignee: nobody → mozilla
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #334654 - Flags: review?(benjamin)

Comment 2

10 years ago
Hrm, I am of the opinion that we should drop support for gcc < 3.4.
(Assignee)

Comment 3

10 years ago
If we _had_ a newer GCC on OS/2 I would agree...
(Reporter)

Comment 4

10 years ago
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!)
(Assignee)

Comment 5

10 years ago
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

10 years ago
Comment on attachment 334654 [details] [diff] [review]
fix

Making the headers so much uglier like this is not an option.
Attachment #334654 - Flags: review?(benjamin) → review-

Comment 7

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

Comment 8

10 years ago
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

10 years ago
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.
(Assignee)

Comment 10

10 years ago
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

10 years ago
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
(Assignee)

Comment 12

10 years ago
(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

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

Comment 14

10 years ago
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.
(Assignee)

Comment 15

10 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.