GC plugin: remove mixin trace override restriction.

GC plugin: remove mixin trace override restriction.

The GC plugin currently insists that a class that is declared as a
mixin instance (i.e., uses USING_GARBAGE_COLLECTED_MIXIN()) must also
declare its own local trace implementation override.

This is an unnecessary restriction to impose:

 - if the class derives from one GarbageCollectedMixin<> instance,
   it will have its virtual trace method in scope.
 - if it inherits from multiple, ambiguity exists and the compiler
   will already complain.
 - if the class declares traceable members of its own, then not providing
   a trace implementation that correctly handles these members will
   be otherwise flagged as an error by the GC plugin.

Remove the static check from the plugin, along with adding a unit test
which verifies that the virtual trace is inherited as expected.

R=thakis,haraken
BUG=444565
Committed: https://crrev.com/e5355493a475e7b32931231e0ee51cc0ca0519a7
Cr-Commit-Position: refs/heads/master@{#364001}

[sof, 2015-12-08 18:33:34]

Description was changed from

==========
GC plugin: remove mixin trace override restriction.

The GC plugin currently insists that a class that is declared as a
mixin instance (i.e., uses USING_GARBAGE_COLLECTED_MIXIN()) must also
declare its own local trace implementation override.

This is an unnecessary restriction to impose:

 - if the class derives from one GarbageCollectedMixin<> instance,
   it will have its virtual trace method in scope.
 - if it inherits from multiple, ambiguity exists and the compiler
   will already complain.
 - if the class declares traceable members of its own, then not providing
   a trace implementation that correctly handles these members will
   be otherwise flagged as an error by the GC plugin.

Remove the static check from the plugin, along with adding a unit test
which verifies that the virtual trace is inherited as expected.

R=
BUG=
==========

to

==========
GC plugin: remove mixin trace override restriction.

The GC plugin currently insists that a class that is declared as a
mixin instance (i.e., uses USING_GARBAGE_COLLECTED_MIXIN()) must also
declare its own local trace implementation override.

This is an unnecessary restriction to impose:

 - if the class derives from one GarbageCollectedMixin<> instance,
   it will have its virtual trace method in scope.
 - if it inherits from multiple, ambiguity exists and the compiler
   will already complain.
 - if the class declares traceable members of its own, then not providing
   a trace implementation that correctly handles these members will
   be otherwise flagged as an error by the GC plugin.

Remove the static check from the plugin, along with adding a unit test
which verifies that the virtual trace is inherited as expected.

R=
BUG=444565
==========

[sof, 2015-12-08 18:50:55]

sigbjornf@opera.com changed reviewers:
+ oilpan-reviews@chromium.org, thakis@chromium.org

[sof, 2015-12-08 18:50:55]

please take a look.

This static check was imposing a needless restriction[1], unconditionally
requiring the definition of a local trace override, i.e.,

 class A : public GC<A>, public SomeGCMixin {
     USING_GARBAGE_COLLECTED_MIXIN(A);
 public:
     // Must be provided.
     DEFINE_INLINE_VIRTUAL_TRACE()
     {
         SomeMixin::trace(visitor);
     }
     //...
 };

A's vtbl slot for |trace| will already get the method that is manually delegated
to here just fine without doing so.

I think we can simply just drop this restriction without taking any special
transition measures -- the bots will be imposing the restriction until the next
Blink roll happens. Anyone building the GC plugin from source until then will
just run into those bots complaining should they come to depend on the lack of
this check. I don't think many will be in that situation besides people
developing the Oilpan infra.

1: confusedly added in r310272 (35290f1b)

[Nico, 2015-12-08 19:27:23]

lgtm as far as I can tell. Maybe an oilpan expert should check that the CL
description / your first email message make sense to them too.

[haraken, 2015-12-08 23:34:23]

LGTM

[sof, 2015-12-09 07:31:44]

Description was changed from

==========
GC plugin: remove mixin trace override restriction.

The GC plugin currently insists that a class that is declared as a
mixin instance (i.e., uses USING_GARBAGE_COLLECTED_MIXIN()) must also
declare its own local trace implementation override.

This is an unnecessary restriction to impose:

 - if the class derives from one GarbageCollectedMixin<> instance,
   it will have its virtual trace method in scope.
 - if it inherits from multiple, ambiguity exists and the compiler
   will already complain.
 - if the class declares traceable members of its own, then not providing
   a trace implementation that correctly handles these members will
   be otherwise flagged as an error by the GC plugin.

Remove the static check from the plugin, along with adding a unit test
which verifies that the virtual trace is inherited as expected.

R=
BUG=444565
==========

to

==========
GC plugin: remove mixin trace override restriction.

The GC plugin currently insists that a class that is declared as a
mixin instance (i.e., uses USING_GARBAGE_COLLECTED_MIXIN()) must also
declare its own local trace implementation override.

This is an unnecessary restriction to impose:

 - if the class derives from one GarbageCollectedMixin<> instance,
   it will have its virtual trace method in scope.
 - if it inherits from multiple, ambiguity exists and the compiler
   will already complain.
 - if the class declares traceable members of its own, then not providing
   a trace implementation that correctly handles these members will
   be otherwise flagged as an error by the GC plugin.

Remove the static check from the plugin, along with adding a unit test
which verifies that the virtual trace is inherited as expected.

R=thakis,haraken
BUG=444565
==========

[sof, 2015-12-09 07:33:44]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2015-12-09 07:34:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1504013004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1504013004/1

[commit-bot: I haz the power, 2015-12-09 07:49:05]

Description was changed from

==========
GC plugin: remove mixin trace override restriction.

The GC plugin currently insists that a class that is declared as a
mixin instance (i.e., uses USING_GARBAGE_COLLECTED_MIXIN()) must also
declare its own local trace implementation override.

This is an unnecessary restriction to impose:

 - if the class derives from one GarbageCollectedMixin<> instance,
   it will have its virtual trace method in scope.
 - if it inherits from multiple, ambiguity exists and the compiler
   will already complain.
 - if the class declares traceable members of its own, then not providing
   a trace implementation that correctly handles these members will
   be otherwise flagged as an error by the GC plugin.

Remove the static check from the plugin, along with adding a unit test
which verifies that the virtual trace is inherited as expected.

R=thakis,haraken
BUG=444565
==========

to

==========
GC plugin: remove mixin trace override restriction.

The GC plugin currently insists that a class that is declared as a
mixin instance (i.e., uses USING_GARBAGE_COLLECTED_MIXIN()) must also
declare its own local trace implementation override.

This is an unnecessary restriction to impose:

 - if the class derives from one GarbageCollectedMixin<> instance,
   it will have its virtual trace method in scope.
 - if it inherits from multiple, ambiguity exists and the compiler
   will already complain.
 - if the class declares traceable members of its own, then not providing
   a trace implementation that correctly handles these members will
   be otherwise flagged as an error by the GC plugin.

Remove the static check from the plugin, along with adding a unit test
which verifies that the virtual trace is inherited as expected.

R=thakis,haraken
BUG=444565
==========

[commit-bot: I haz the power, 2015-12-09 07:49:05]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-12-09 07:49:57]

Description was changed from

==========
GC plugin: remove mixin trace override restriction.

The GC plugin currently insists that a class that is declared as a
mixin instance (i.e., uses USING_GARBAGE_COLLECTED_MIXIN()) must also
declare its own local trace implementation override.

This is an unnecessary restriction to impose:

 - if the class derives from one GarbageCollectedMixin<> instance,
   it will have its virtual trace method in scope.
 - if it inherits from multiple, ambiguity exists and the compiler
   will already complain.
 - if the class declares traceable members of its own, then not providing
   a trace implementation that correctly handles these members will
   be otherwise flagged as an error by the GC plugin.

Remove the static check from the plugin, along with adding a unit test
which verifies that the virtual trace is inherited as expected.

R=thakis,haraken
BUG=444565
==========

to

==========
GC plugin: remove mixin trace override restriction.

The GC plugin currently insists that a class that is declared as a
mixin instance (i.e., uses USING_GARBAGE_COLLECTED_MIXIN()) must also
declare its own local trace implementation override.

This is an unnecessary restriction to impose:

 - if the class derives from one GarbageCollectedMixin<> instance,
   it will have its virtual trace method in scope.
 - if it inherits from multiple, ambiguity exists and the compiler
   will already complain.
 - if the class declares traceable members of its own, then not providing
   a trace implementation that correctly handles these members will
   be otherwise flagged as an error by the GC plugin.

Remove the static check from the plugin, along with adding a unit test
which verifies that the virtual trace is inherited as expected.

R=thakis,haraken
BUG=444565

Committed: https://crrev.com/e5355493a475e7b32931231e0ee51cc0ca0519a7
Cr-Commit-Position: refs/heads/master@{#364001}
==========

[commit-bot: I haz the power, 2015-12-09 07:49:57]

Patchset 1 (id:??) landed as
https://crrev.com/e5355493a475e7b32931231e0ee51cc0ca0519a7
Cr-Commit-Position: refs/heads/master@{#364001}