Add constructor for CPPLanguage (which inherits from Language) to placate CLang

RESOLVED FIXED in mozilla2.0b10

Status

()

defect
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: espindola, Assigned: espindola)

Tracking

Trunk
mozilla2.0b10
x86_64
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Posted patch patchSplinter Review
CPPLanguage is missing a user defined constructor,
but in language.cc 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 #501238 - Attachment is patch: true
Attachment #501238 - Attachment mime type: application/octet-stream → text/plain
Attachment #501238 - Flags: review?(ted.mielczarek)

Comment 1

9 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 CPPLanguage (which inherits from Language) to placate LLVM

Updated

9 years ago
Summary: Add constructor for CPPLanguage (which inherits from Language) to placate LLVM → Add constructor for CPPLanguage (which inherits from Language) to placate CLang
Comment on attachment 501238 [details] [diff] [review]
patch

Jim wrote all this code.
Attachment #501238 - Flags: review?(ted.mielczarek) → review?(jimb)
Assignee: nobody → respindola

Comment 3

9 years ago
Comment on attachment 501238 [details] [diff] [review]
patch

This needs to go upstream to breakpad. I don't want to accept it locally for just a compiler fix.
Yeah, I'm fine with getting r+ on these in bugzilla and pushing them all upstream at once, though, since they're all pretty trivial fixes.
so, on whose plate is this? If mine should I send the code for review in breakpad first or finish this review first?
Doesn't really matter, as long as it gets reviewed somewhere. We can land it in both places at the same time. Jim and I are both Breakpad peers, and the Breakpad project isn't really nitpicky about where reviews happen. (The Google devs do reviews in the Chromium review tool all the time.)

Updated

9 years ago
Attachment #501238 - Flags: review?(jimb) → review+

Updated

8 years ago
Attachment #501238 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/b0c917ad881b
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b10
Landed upstream:
http://code.google.com/p/google-breakpad/source/detail?r=764

Rafael: can you close the Breakpad review issue:
http://breakpad.appspot.com/249001/show ? (Click "Edit Issue" then there's a "Closed" checkbox.) Only the person who created an issue can edit it.
You need to log in before you can comment on or make changes to this bug.