812531 - Uses of TYPE_SPECIFIC_SCOPED_POINTER_TEMPLATE break when compiled with Clang

Status: RESOLVED FIXED

(Core :: MFBT, defect, P1)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: Make MOZ_TYPE_SPECIFIC_SCOPED_POINTER_TEMPLATE comply with two-phase template name lookup rules, as clang requires

+++ This bug was initially created as a clone of Bug #767240 +++

Clang strictly enforces the C++ rules for two-phase name lookup in templates, and TYPE_SPECIFIC_SCOPED_POINTER_TEMPLATE violates the rules.

This is blocking a P1 basecamp blocker.

Comment 1

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 682490 [details] [diff] [review]
Make MOZ_TYPE_SPECIFIC_SCOPED_POINTER_TEMPLATE comply with two-phase template name lookup rules, as clang requires

r=khuey

Comment 2

[Lucas Adamski [:ladamski]]

Necessary for app signing (bug 772365).

Comment 3

[Jeff Walden [:Waldo]]

Would you care to explain what the issue is this actually fixes?  I have too little time these days, but if I'd been able to get to this in time I'd probably just have rubberstamped it, without understanding exactly what makes it necessary in conformant compilers.

Comment 4

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Jeff, it is basically an instance of this problem:
http://blog.llvm.org/2009/12/dreaded-two-phase-name-lookup.html.

clang will refuse to compile the TypeSpecificScopedPointerTraits::release template because TypeSpecificDelete wasn't declared before the definition of the template.

I did not notice the problem before because I had not actually checked in any uses of the template, and I'd only tested uses of the template on Windows.

Comment 5

[Jeff Walden [:Waldo]]

Ah, that makes a lot of sense.  Thanks for the pointer!

Comment 6

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 682490 [details] [diff] [review]
Make MOZ_TYPE_SPECIFIC_SCOPED_POINTER_TEMPLATE comply with two-phase template name lookup rules, as clang requires

https://hg.mozilla.org/integration/mozilla-inbound/rev/1ca9218271b9

[Approval Request Comment]
User impact if declined: Prerequisite of packaged app support for B2G
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: None

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1ca9218271b9

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 682490 [details] [diff] [review]
Make MOZ_TYPE_SPECIFIC_SCOPED_POINTER_TEMPLATE comply with two-phase template name lookup rules, as clang requires

Approval isn't currently required to uplift bb+ bugs.

Comment 9

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/70ce9517d91f
https://hg.mozilla.org/releases/mozilla-beta/rev/a1d710de7c6d

Comment 10

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Fix bustage caused by bad automatic merge:
https://hg.mozilla.org/releases/mozilla-aurora/rev/4005d68e2d83
https://hg.mozilla.org/releases/mozilla-beta/rev/2e2ef6b0d646