error: 'HasDangerousPublicDestructor' is not a template

RESOLVED FIXED in Thunderbird 42.0

Status

defect
--
blocker
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: philip.chee, Assigned: philip.chee)

Tracking

({dogfood})

Trunk
Thunderbird 42.0

Firefox Tracking Flags

(firefox42 affected, thunderbird42 fixed, seamonkey2.39 fixed)

Details

User Story

> 17:49	RattyAway	Anyone understand C++ ? Bug 1183729
> 17:49	firebot	https://bugzil.la/1183729 — NEW, nobody%mozilla.org — error: 'HasDangerousPublicDestructor' is not a template
> 17:50	RattyAway	This explains how to fix things: https://bugzilla.mozilla.org/show_bug.cgi?id=1028132#c4
> 17:50	firebot	Bug 1028132 — FIXED, michael%thelayzells.com — Fix all the temporarily whitelisted dangerous public destructors of refcounted classes
> 17:50	RattyAway	but only if you know C++
> 17:50	jcranmer|away	RattyAway: ISTR that one or two of the dangerous public destructors are Really Hard To Fix™
> 17:51	jcranmer|away	(specifically, there's one or two libmime gross hacks that's used as a POD in libmime but is also exposed as an xpcom object)
> 17:52	RattyAway	jcranmer|away: I had a go at it but that just makes new error messages popup somewhere in mime code.
> 17:52	jcranmer|away	RattyAway: that's why it's Really Hard To Fix™
> 17:52	jcranmer|away	I think I got halfway on one of them once

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
https://treeherder.mozilla.org/logviewer.html#?job_id=21079&repo=comm-central

In file included from ../../../../dist/include/mimemoz2.h:15:0,
from /builds/slave/tb-c-cen-lx-ntly-0000000000000/build/mailnews/mime/cthandlers/pgpmime/nsPgpMimeProxy.cpp:22:
../../../../dist/include/nsMsgAttachmentData.h:101:20: error: 'HasDangerousPublicDestructor' is not a template
../../../../dist/include/nsMsgAttachmentData.h:102:1: error: explicit specialization of non-template 'mozilla::HasDangerousPublicDestructor'
../../../../dist/include/nsMsgAttachmentData.h:105:20: error: 'HasDangerousPublicDestructor' is not a template
../../../../dist/include/nsMsgAttachmentData.h:105:66: error: 'mozilla::HasDangerousPublicDestructor' is not a template type
Severity: major → blocker
Keywords: dogfood, regression
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
(Assignee)

Comment 5

4 years ago
> 17:49	RattyAway	Anyone understand C++ ? Bug 1183729
> 17:49	firebot	https://bugzil.la/1183729 — NEW, nobody%mozilla.org — error: 'HasDangerousPublicDestructor' is not a template
> 17:50	RattyAway	This explains how to fix things: https://bugzilla.mozilla.org/show_bug.cgi?id=1028132#c4
> 17:50	firebot	Bug 1028132 — FIXED, michael%thelayzells.com — Fix all the temporarily whitelisted dangerous public destructors of refcounted classes
> 17:50	RattyAway	but only if you know C++
> 17:50	jcranmer|away	RattyAway: ISTR that one or two of the dangerous public destructors are Really Hard To Fix™
> 17:51	jcranmer|away	(specifically, there's one or two libmime gross hacks that's used as a POD in libmime but is also exposed as an xpcom object)
> 17:52	RattyAway	jcranmer|away: I had a go at it but that just makes new error messages popup somewhere in mime code.
> 17:52	jcranmer|away	RattyAway: that's why it's Really Hard To Fix™
> 17:52	jcranmer|away	I think I got halfway on one of them once
User Story: (updated)
(Assignee)

Comment 6

4 years ago
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=ab9bf00b6478
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
(Assignee)

Updated

4 years ago
Assignee: philip.chee → nobody
Status: ASSIGNED → NEW
(Assignee)

Comment 8

4 years ago
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
Attachment #8634252 - Flags: review?(rkent)
(Assignee)

Updated

4 years ago
Attachment #8634252 - Flags: review?(rkent) → review?(neil)
(Assignee)

Updated

4 years ago
Attachment #8634252 - Flags: review?(Pidgeot18)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)
Comment hidden (Legacy TBPL/Treeherder Robot)

Comment 21

4 years ago
The posted patch allowed me to build C-C TB successfully.
(I will run mozmill and xpcshell test.)
Comment on attachment 8634252 [details] [diff] [review]
Patch v1.0 Attempted fix.

Review of attachment 8634252 [details] [diff] [review]:
-----------------------------------------------------------------

r+, but only if the changes below are made.

::: db/mork/src/morkEnv.h
@@ +201,5 @@
>    { morkNode::SlotStrongNode((morkNode*) me, ev, (morkNode**) ioSlot); }
>  };
>  
> +#ifdef MOZ_IS_DESTRUCTIBLE
> +#define MOZ_ASSERT_TYPE_OK_FOR_REFCOUNTING(X) \

You need to #undef MOZ_ASSERT_TYPE_OK_FOR_REFCOUNTING first, otherwise you'll get oodles of warning spam (and I'm surprised it's not a compiler error on some platform.

@@ +203,5 @@
>  
> +#ifdef MOZ_IS_DESTRUCTIBLE
> +#define MOZ_ASSERT_TYPE_OK_FOR_REFCOUNTING(X) \
> +  static_assert(!MOZ_IS_DESTRUCTIBLE(X) || \
> +                mozilla::MorkHasDangerousPublicDestructor<X>::value, \

You can replace the mozilla::MorkHasDangerousPublicDestructor<X>::value check with mozilla::IsSame<X, morkEnv>::value, and similarly for the mailnews/compose check.

@@ +208,5 @@
> +                "Reference-counted class " #X " should not have a public destructor. " \
> +                "Try to make this class's destructor non-public. If that is really " \
> +                "not possible, you can whitelist this class by providing a " \
> +                "HasDangerousPublicDestructor specialization for it."); \
> +  static_assert(!mozilla::MorkHasDangerousPublicDestructor<X>::value || \

... and trash this static_assert.

@@ +220,1 @@
>  namespace mozilla {

... and this entire namespace block.
Attachment #8634252 - Flags: review?(Pidgeot18) → review+
(Assignee)

Comment 23

4 years ago
http://hg.mozilla.org/comm-central/rev/5f78a96a1486
http://hg.mozilla.org/comm-central/rev/3ec8d1385b80 (It would help if I qrefreshed first)
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Keywords: regression
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 42.0
(Assignee)

Updated

4 years ago
Attachment #8634252 - Flags: review?(neil)
You need to log in before you can comment on or make changes to this bug.