error: 'HasDangerousPublicDestructor' is not a template

RESOLVED FIXED in Thunderbird 42.0

Status

MailNews Core
Backend
--
blocker
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Philip Chee, Assigned: Philip Chee)

Tracking

({dogfood})

Trunk
Thunderbird 42.0
dogfood

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

2 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

Updated

2 years ago
Severity: major → blocker
Keywords: dogfood, regression
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 5

2 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

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

Updated

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

Comment 7

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=8f3a0c04964f
(Assignee)

Comment 8

2 years ago
Created attachment 8634252 [details] [diff] [review]
Patch v1.0 Attempted fix.
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
Attachment #8634252 - Flags: review?(rkent)
(Assignee)

Updated

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

Updated

2 years ago
Attachment #8634252 - Flags: review?(Pidgeot18)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)

Comment 21

2 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

2 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: 2 years ago
status-seamonkey2.39: --- → fixed
status-thunderbird42: --- → fixed
Keywords: regression
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 42.0
(Assignee)

Updated

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