blink_gc_plugin: warn of unused trace methods to stack allocated classes.

blink_gc_plugin: warn of unused trace methods to stack allocated classes.

A STACK_ALLOCATED()-annotated class does not need a trace method; issue
a warning if encountered.

R=haraken
BUG=689874
Review-Url: https://codereview.chromium.org/2685583002
Cr-Commit-Position: refs/heads/master@{#449046}
Committed: https://chromium.googlesource.com/chromium/src/+/59dc1fe4df1db90553f572d20cf2b732438063b2

[sof, 2017-02-07 14:25:21]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-07 14:25:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sof, 2017-02-07 14:27:35]

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

[sof, 2017-02-07 14:27:35]

please take a look.

overall it is worth warning of dead code, hence added.

[commit-bot: I haz the power, 2017-02-07 14:41:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-07 14:41:07]

Dry run: This issue passed the CQ dry run.

[haraken, 2017-02-07 15:24:29]

LGTM

[sof, 2017-02-07 15:44:10]

thakis@, hans@: adding a new warning to the GC plugin, but for a condition
that's on the rare side. We could an option for it that's default-disabled and
then do the steps reqd to turn it on post-roll etc, but I'm wondering if we can
avoid those extra steps here?

Up to you; I can definitely verify that ToT builds cleanly wrt this warning up
until the roll goes out.

[Nico, 2017-02-07 15:46:17]

Please add a flag. We haven't updated the compiler in way too long, which
means the next update will be bumpy already.

On Tue, Feb 7, 2017 at 10:44 AM, <sigbjornf@opera.com> wrote:

> thakis@, hans@: adding a new warning to the GC plugin, but for a condition
> that's on the rare side. We could an option for it that's default-disabled
> and
> then do the steps reqd to turn it on post-roll etc, but I'm wondering if
> we can
> avoid those extra steps here?
>
> Up to you; I can definitely verify that ToT builds cleanly wrt this
> warning up
> until the roll goes out.
>
> https://codereview.chromium.org/2685583002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sof, 2017-02-08 07:44:16]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-08 07:44:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sof, 2017-02-08 07:44:42]

Description was changed from

==========
blink_gc_plugin: warn of unused trace methods to stack allocated classes.

A STACK_ALLOCATED()-annotated class does not need a trace method; issue
a warning if encountered.

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

to

==========
blink_gc_plugin: warn of unused trace methods to stack allocated classes.

A STACK_ALLOCATED()-annotated class does not need a trace method; issue
a warning if encountered.

R=haraken
BUG=689874
==========

[sof, 2017-02-08 07:48:51]

On 2017/02/07 15:46:17, Nico wrote:
> Please add a flag. We haven't updated the compiler in way too long, which
> means the next update will be bumpy already.
> 

ok, added + created issue 689874 (is there a tracking bug yet for that next
roll?).

Will add a Source/BUILD.gn option once it safely lands, and start the process of
enabling the warning.

[commit-bot: I haz the power, 2017-02-08 07:52:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-08 07:52:18]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[hans, 2017-02-08 16:17:03]

On 2017/02/08 07:48:51, sof wrote:
> On 2017/02/07 15:46:17, Nico wrote:
> > Please add a flag. We haven't updated the compiler in way too long, which
> > means the next update will be bumpy already.
> > 
> 
> ok, added + created issue 689874 (is there a tracking bug yet for that next
> roll?).

It's crbug.com/685244

[sof, 2017-02-08 18:31:42]

On 2017/02/08 16:17:03, hans wrote:
> On 2017/02/08 07:48:51, sof wrote:
> > On 2017/02/07 15:46:17, Nico wrote:
> > > Please add a flag. We haven't updated the compiler in way too long, which
> > > means the next update will be bumpy already.
> > > 
> > 
> > ok, added + created issue 689874 (is there a tracking bug yet for that next
> > roll?).
> 
> It's crbug.com/685244

thanks, added the dependency.

[sof, 2017-02-08 18:31:47]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2017-02-08 18:31:47]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2685583002/#ps40001
(title: "rebased upto r449038")

[commit-bot: I haz the power, 2017-02-08 18:32:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-08 18:49:50]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1486578707446900,
"parent_rev": "7cb979b7a35fae74baaf0d36b5f33b5ccab74a31", "commit_rev":
"59dc1fe4df1db90553f572d20cf2b732438063b2"}

[commit-bot: I haz the power, 2017-02-08 18:50:23]

Description was changed from

==========
blink_gc_plugin: warn of unused trace methods to stack allocated classes.

A STACK_ALLOCATED()-annotated class does not need a trace method; issue
a warning if encountered.

R=haraken
BUG=689874
==========

to

==========
blink_gc_plugin: warn of unused trace methods to stack allocated classes.

A STACK_ALLOCATED()-annotated class does not need a trace method; issue
a warning if encountered.

R=haraken
BUG=689874

Review-Url: https://codereview.chromium.org/2685583002
Cr-Commit-Position: refs/heads/master@{#449046}
Committed:
https://chromium.googlesource.com/chromium/src/+/59dc1fe4df1db90553f572d20cf2...
==========

[commit-bot: I haz the power, 2017-02-08 18:50:24]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/59dc1fe4df1db90553f572d20cf2...