Closed Bug 451278 Opened 13 years ago Closed 13 years ago

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

Categories

(Core :: XPCOM, defect)

x86
OS/2
defect
Not set
blocker

Tracking

()

RESOLVED WONTFIX

People

(Reporter: wuno, Assigned: mozilla)

References

Details

Attachments

(1 file)

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.
Blocks: 265534
Severity: normal → blocker
Version: unspecified → Trunk
Attached patch fixSplinter Review
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)
Hrm, I am of the opinion that we should drop support for gcc < 3.4.
If we _had_ a newer GCC on OS/2 I would agree...
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!)
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 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-
in reply to Comment #5
The GCC bug entry is http://gcc.gnu.org/bugzilla/show_bug.cgi?id=6992
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?
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.
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.
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
(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.
I thought I made it clear why this patch was not acceptable: it adds ifdefs which make the code harder to read and maintain.
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.
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
Closed: 13 years ago
Resolution: --- → WONTFIX
Component: String → XPCOM
You need to log in before you can comment on or make changes to this bug.