Add constructor for nsDebugImpl, nsTraceRefcntImpl, EmptyEnumeratorImpl, and nsSimpleUnicharStreamFactory to placate CLang

RESOLVED FIXED

Status

()

Core
XPCOM
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: espindola, Assigned: espindola)

Tracking

Trunk
x86_64
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Created attachment 501245 [details] [diff] [review]
patch

nsSimpleUnicharStreamFactory is missing a user defined constructor,
but in nsUnicharInputStream.cpp a const variable of this type is defined.

This is not valid c++. For more information see "Default initialization of
const variable of a class type requires user-defined default constructor" in
http://clang.llvm.org/compatibility.html#c++
Attachment #501245 - Attachment is patch: true
Attachment #501245 - Attachment mime type: application/octet-stream → text/plain
Attachment #501245 - Flags: review?(benjamin)
Blocks: 574346

Comment 1

7 years ago
The reporter's summary and initial comment were both lame. I'm merely adjusting
the summary and providing a better link. I am not passing judgement on the
quality of the bug report.

http://clang.llvm.org/compatibility.html#default_init_const
Summary: Missing constructor → Add constructor for nsDebugImpl, nsTraceRefcntImpl, EmptyEnumeratorImpl, and nsSimpleUnicharStreamFactory to placate CLang
Assignee: nobody → respindola
Just as with the other bug, the missing constructor is intentional so that GCC does not emit a initialization function. Absent a detailed explanation from a language lawyer, I don't think I want to accept this change.
Given the discussion on bug 623123, are you ok with it?
Duplicate of this bug: 614789
Attachment #501245 - Flags: review?(benjamin) → review+
Depends on: 610267
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/ca41c5663999
Status: NEW → RESOLVED
Last Resolved: 6 years ago
No longer depends on: 610267
Keywords: checkin-needed
Resolution: --- → FIXED
Duplicate of this bug: 645469

Comment 7

6 years ago
(In reply to comment #2)
> Just as with the other bug, the missing constructor is intentional so that GCC
> does not emit a initialization function. Absent a detailed explanation from a
> language lawyer, I don't think I want to accept this change.

A bit late, but language lawyer here, see http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44499#c2 for chapter and verse. As pointed out by some of the dups of this bug, GCC 4.6 rejects this too, see the note I added to http://gcc.gnu.org/gcc-4.6/changes.html#cplusplus

Comment 8

6 years ago
I see that the commit adds empty user-defined constructors. Did anyone consider the alternative of adding an initializer instead, as I suggested in the GCC 4.6 changes and in Bug 645469 c5?
That might avoid emitting an initialization function.
(In reply to comment #8)
> I see that the commit adds empty user-defined constructors. Did anyone consider
> the alternative of adding an initializer instead, as I suggested in the GCC 4.6
> changes and in Bug 645469 c5?
> That might avoid emitting an initialization function.

I don't think it makes any difference in the generated code.

Updated

6 years ago
requesting approval for mozilla-2.0 branch:  with Fedora 15, gcc 4.6.0, FF4 will not compile.  #developers pointed me to this bug.
status2.0: --- → ?
the 2.0 branch doesn't have any planned updates, what is the purpose of the nomination?
status2.0: ? → ---
I wonder why this is marked as resolved fix?  In Fedora 15 it is still a problem, was the patch applied upstream and then not brought into Fedora?  I'm trying to rebuild sunbird, and this bug in xulrunner is breaking the sunbird build.  I just created an updated version of the xulrunner rpm using this patch to fix the problem, but if there is something upstream I can grab it is probably better to use that.
You need to log in before you can comment on or make changes to this bug.