1289608 - WrapperFactory.cpp:348:42: error: instantiation of variable 'xpc::XrayWrapper<js::CrossCompartmentWrapper, xpc::XPCWrappedNativeXrayTraits>::singleton' required here, but no definition is available [-Werror,-Wundefined-var-template]

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Daniel Holbert [:dholbert]]

I'm trying to build with clang 3.9 today (on Linux64), and I got this build error (from a build-warning being promoted to an error due to --enable-warnings-as-errors in my mozconfig):

{
 3:57.83 /scratch/work/builds/mozilla-inbound/mozilla/js/xpconnect/wrappers/WrapperFactory.cpp:348:42: error: instantiation of variable 'xpc::XrayWrapper<js::CrossCompartmentWrapper, xpc::XPCWrappedNativeXrayTraits>::singleton' required here, but no definition is available [-Werror,-Wundefined-var-template]
 3:57.83             return &PermissiveXrayXPCWN::singleton;
 3:57.83                                          ^
 3:57.83 /scratch/work/builds/mozilla-inbound/mozilla/js/xpconnect/wrappers/XrayWrapper.h:463:30: note: forward declaration of template entity is here
 3:57.83     static const XrayWrapper singleton;
 3:57.83                              ^
 3:57.83 /scratch/work/builds/mozilla-inbound/mozilla/js/xpconnect/wrappers/WrapperFactory.cpp:348:42: note: add an explicit instantiation declaration to suppress this warning if 'xpc::XrayWrapper<js::CrossCompartmentWrapper, xpc::XPCWrappedNativeXrayTraits>::singleton' is explicitly instantiated in another translation unit
 3:57.83             return &PermissiveXrayXPCWN::singleton;
 3:57.83                                          ^
 3:57.84 1 error generated.
}

Comment 1

[Daniel Holbert [:dholbert]]

Hoping maybe jandem or bholley can take a look & suggest a fix here, since they've touched this file/component in the recent past...

If you'd like to use clang 3.9 locally to trigger this, you can probably find binaries somewhere, or you can build clang 3.9 yourself by e.g. following the 8-line script in our MDN page, as I discussed here:
 https://bugzilla.mozilla.org/show_bug.cgi?id=1285668#c1 )

Comment 2

[Daniel Holbert [:dholbert]]

(and anyone working on this with clang 3.9 may find it useful to apply most of my hackaround patch from bug 1285668 [everything but the tweak for this directory], to avoid other warnings-as-errors issues. Or alternately, just build without --enable-warnings-as-errors in your mozconfig, and watch for this bug's warning text in your buildspew.)

Comment 3

[Hsin-Yi Tsai (she/her) [:hsinyi]]

per comment 1, do you have ideas about this bug, jandem or bholley? Thanks!

Comment 4

[Bobby Holley (:bholley)]

This code is doing explicit template instantiation for various template parameters in a C++ file so that other consumers can find those symbols at link time (rather than having to #include the template implementation themselves). It sounds like we need an "explicit instantiation declaration", whatever that is.

Comment 5

[Andrew Overholt [:overholt]]

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #4)
> This code is doing explicit template instantiation for various template
> parameters in a C++ file so that other consumers can find those symbols at
> link time (rather than having to #include the template implementation
> themselves). It sounds like we need an "explicit instantiation declaration",
> whatever that is.

Botond, do you know about this?

Comment 6

[Botond Ballo [:botond]]

The "explicit instantiation declaration" has the form:

  extern template class ClassName<Args>;

It doesn't actually instantiate ClassName<Args>, but it tells the compiler "I promise another translation unit will instantiate ClassName<Args>".

In this case, declarations of this form for various instantiations of XrayWrapper<...> need to be present before the uses of XrapWrapper<...>::singleton in WrapperFactory.cpp (XrayWrapper.h is probably a good place for them).

The code has another problem, which clang will start complaining about as soon as you introduce the explicit instantiation declarations. These declarations in XrayWrapper.cpp [1] are actually declaring *explicit specializations* of the XrayWrapper<...>::singleton, which don't mix well with explicit instantiations.

The correct thing to do here is to provide a definition for the static member of the primary template:

  template <typename Base, typename Traits>
  XrayWrapper<Base, Traits> XrayWrapper<Base, Traits>::singleton(0);

and then proceed with the explicit instantiations of the class:

  template class PermissiveXrayXPCWN;
  template class SecurityXrayXPCWN;
  ...

which will generate corresponding instantiations for the static data member as well.

(Tangentially related question: why are things like PermissiveXrayXPCWN macros instead of typedefs?)

[1] http://searchfox.org/mozilla-central/rev/1112b7a5222b71a3b5b68bd531f50ded6bcbc770/js/xpconnect/wrappers/XrayWrapper.cpp#2408

Comment 7

[Botond Ballo [:botond]]

Attachment: Bug 1289608 - Provide explicit instantiation declarations for the various instantiations of XrapWrapper.

Review commit: https://reviewboard.mozilla.org/r/68060/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/68060/

Comment 8

[Botond Ballo [:botond]]

Attachment: Bug 1289608 - Define XrayWrapper<...>::singleton for all instantiations, instead of defining specializations for specific ones.

Review commit: https://reviewboard.mozilla.org/r/68062/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/68062/

Comment 9

[Botond Ballo [:botond]]

The posted patches implement the changes suggested in comment 6. They compile for me locally with clang 3.9, and here's a Try push to verify that they compile in automation:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ed4b46257c2

Comment 10

[Botond Ballo [:botond]]

(In reply to Daniel Holbert [:dholbert] (mostly OOTO until Aug 9th) from comment #1)
> If you'd like to use clang 3.9 locally to trigger this, you can probably
> find binaries somewhere

(For Debian/Ubuntu, I recommend http://apt.llvm.org/).

Comment 11

[Bobby Holley (:bholley)]

Comment on attachment 8776162 [details]
Bug 1289608 - Provide explicit instantiation declarations for the various instantiations of XrapWrapper.

https://reviewboard.mozilla.org/r/68060/#review65194

Comment 12

[Bobby Holley (:bholley)]

Comment on attachment 8776163 [details]
Bug 1289608 - Define XrayWrapper<...>::singleton for all instantiations, instead of defining specializations for specific ones.

https://reviewboard.mozilla.org/r/68062/#review65196

r+ if it compiles. Thanks for fixing this.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/19417c63e17e
Provide explicit instantiation declarations for the various instantiations of XrapWrapper. r=bholley
https://hg.mozilla.org/integration/autoland/rev/891c4b4e2797
Define XrayWrapper<...>::singleton for all instantiations, instead of defining specializations for specific ones. r=bholley

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/19417c63e17e
https://hg.mozilla.org/mozilla-central/rev/891c4b4e2797

Comment 15

[Daniel Holbert [:dholbert]]

Thanks, everyone!