declare extern template classes sooner

declare extern template classes sooner

This avoids a gcc warning that is promoted to an error:
error: type attributes ignored after type is already defined [-Werror=attributes]

(Tested with gcc 4.8.2-19ubuntu1.)

BUG=414363
Committed: https://crrev.com/83d5b708e432a573da1245316882ecd7df080cf3
Cr-Commit-Position: refs/heads/master@{#299532}

[Mostyn Bramley-Moore, 2014-10-14 18:45:46]

mostynb@opera.com changed reviewers:
+ jorgelo@chromium.org, leecam@chromium.org, mdempsky@chromium.org

[Mostyn Bramley-Moore, 2014-10-14 18:46:33]

@mdempsky, leecam, Jorge: please take a look at this build fixup for
https://codereview.chromium.org/635223006

[mdempsky, 2014-10-14 18:48:59]

not lgtm

That SANDBOX_EXPORT declaration is needed *somewhere* to make sure the extern
template definitions are available outside of the seccomp-bpf shared library
when doing a component build.

Where's the compiler think the previous declaration was?  Maybe we need to move
SANDBOX_EXPORT there.

[Mostyn Bramley-Moore, 2014-10-14 18:51:04]

On 2014/10/14 18:48:59, mdempsky wrote:
> not lgtm
> 
> That SANDBOX_EXPORT declaration is needed *somewhere* to make sure the extern
> template definitions are available outside of the seccomp-bpf shared library
> when doing a component build.
> 
> Where's the compiler think the previous declaration was?  Maybe we need to
move
> SANDBOX_EXPORT there.

I noticed this soon after I hit send.  Poking around now to find the proper
fix...

[mdempsky, 2014-10-14 18:55:51]

On 2014/10/14 18:51:04, Mostyn Bramley-Moore wrote:
> I noticed this soon after I hit send.  Poking around now to find the proper
> fix...

SGTM, thanks.

The easiest quick fix might be to move the "extern template" statements up to
just below the "Forward declarations of classes".  It'll just look a bit ugly
because we'll need to close the namespace context and reopen it.

Tangentially, what compiler and version are you using?

[Mostyn Bramley-Moore, 2014-10-14 18:57:45]

> The easiest quick fix might be to move the "extern template" statements up to
> just below the "Forward declarations of classes".  It'll just look a bit ugly
> because we'll need to close the namespace context and reopen it.
> 
> Tangentially, what compiler and version are you using?

The default toolchain in ubuntu 14.04: gcc 4.8.2-19ubuntu1.

[Mostyn Bramley-Moore, 2014-10-14 19:04:34]

> > The easiest quick fix might be to move the "extern template" statements up
to
> > just below the "Forward declarations of classes".  It'll just look a bit
ugly
> > because we'll need to close the namespace context and reopen it.

This WFM locally, done in patchset 2.

[mdempsky, 2014-10-14 19:07:10]

lgtm

Ugly, but it'll do for now. Thanks!

[Mostyn Bramley-Moore, 2014-10-14 19:08:14]

The CQ bit was checked by mostynb@opera.com

[Mostyn Bramley-Moore, 2014-10-14 19:09:26]

np.  I tried to clean up the CL description a bit too.

[commit-bot: I haz the power, 2014-10-14 19:09:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/643703003/20001

[mdempsky, 2014-10-14 19:20:08]

On 2014/10/14 19:09:26, Mostyn Bramley-Moore wrote:
> np.  I tried to clean up the CL description a bit too.

LGTM

For posterity, it appears the issue is when GCC sees the templates like:

    template <typename T>
    class Arg {
     public:
      ...
      friend BoolExpr operator==(const Arg& lhs, T rhs) { ... }
      ...
    };

it instantiates BoolExpr's definition for use in operator=='s definition.  Then
when we do the "extern template class SANDBOX_EXPORT BoolExpr;" below, the
SANDBOX_EXPORT attribute is ignored because BoolExpr has already been
instantiated/defined.

On the other hand, Clang seems happy to still pick up the SANDBOX_EXPORT
attribute.  I'll investigate why and alert the GCC/Clang guys about the
discrepancy.

[commit-bot: I haz the power, 2014-10-14 19:57:38]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-10-14 20:01:45]

Patchset 2 (id:??) landed as
https://crrev.com/83d5b708e432a573da1245316882ecd7df080cf3
Cr-Commit-Position: refs/heads/master@{#299532}