Closed Bug 1289608 Opened 3 years ago Closed 3 years ago

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]

Categories

(Core :: XPConnect, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: dholbert, Assigned: botond)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

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.
}
Blocks: 1285668
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 )
(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.)
per comment 1, do you have ideas about this bug, jandem or bholley? Thanks!
Flags: needinfo?(jdemooij)
Flags: needinfo?(bobbyholley)
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.
Flags: needinfo?(bobbyholley)
(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?
Flags: needinfo?(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
Flags: needinfo?(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
(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 on attachment 8776162 [details]
Bug 1289608 - Provide explicit instantiation declarations for the various instantiations of XrapWrapper.

https://reviewboard.mozilla.org/r/68060/#review65194
Attachment #8776162 - Flags: review+
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.
Attachment #8776163 - Flags: review+
Assignee: nobody → botond
Pushed by bballo@mozilla.com:
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
Flags: needinfo?(jdemooij)
https://hg.mozilla.org/mozilla-central/rev/19417c63e17e
https://hg.mozilla.org/mozilla-central/rev/891c4b4e2797
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Thanks, everyone!
You need to log in before you can comment on or make changes to this bug.