blink_gc_plugin: always enable warn_stack_allocated_trace_method check.

blink_gc_plugin: always enable warn_stack_allocated_trace_method check.

R=haraken,thakis
BUG=689864
Review-Url: https://codereview.chromium.org/2730673003
Cr-Commit-Position: refs/heads/master@{#454307}
Committed: https://chromium.googlesource.com/chromium/src/+/76004798b3237c002bd84d5bfb6ff524d9ebb248

[sof, 2017-03-02 14:05:54]

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

[commit-bot: I haz the power, 2017-03-02 14:06:10]

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

[sof, 2017-03-02 14:12:02]

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

[sof, 2017-03-02 14:12:03]

please take a look.

step 3 (of 5) - unit tests locally confirmed to pass.

[Nico, 2017-03-02 14:18:21]

lgtm, but

https://codereview.chromium.org/2730673003/diff/1/tools/clang/blink_gc_plugin...
File tools/clang/blink_gc_plugin/BlinkGCPluginOptions.h (right):

https://codereview.chromium.org/2730673003/diff/1/tools/clang/blink_gc_plugin...
tools/clang/blink_gc_plugin/BlinkGCPluginOptions.h:17: bool
warn_stack_allocated_trace_method = true;
I think you can remove this completely and pretend it's true everywhere where it
currently is (and accept the flag but make it a no-op)

[commit-bot: I haz the power, 2017-03-02 14:22:48]

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

[commit-bot: I haz the power, 2017-03-02 14:22:48]

Dry run: This issue passed the CQ dry run.

[sof, 2017-03-02 14:23:21]

https://codereview.chromium.org/2730673003/diff/1/tools/clang/blink_gc_plugin...
File tools/clang/blink_gc_plugin/BlinkGCPluginOptions.h (right):

https://codereview.chromium.org/2730673003/diff/1/tools/clang/blink_gc_plugin...
tools/clang/blink_gc_plugin/BlinkGCPluginOptions.h:17: bool
warn_stack_allocated_trace_method = true;
On 2017/03/02 14:18:21, Nico wrote:
> I think you can remove this completely and pretend it's true everywhere where
it
> currently is (and accept the flag but make it a no-op)

Does it matter much? we'll have to be back one more time in any case.

[Nico, 2017-03-02 14:26:50]

https://codereview.chromium.org/2730673003/diff/1/tools/clang/blink_gc_plugin...
File tools/clang/blink_gc_plugin/BlinkGCPluginOptions.h (right):

https://codereview.chromium.org/2730673003/diff/1/tools/clang/blink_gc_plugin...
tools/clang/blink_gc_plugin/BlinkGCPluginOptions.h:17: bool
warn_stack_allocated_trace_method = true;
On 2017/03/02 14:23:21, sof wrote:
> On 2017/03/02 14:18:21, Nico wrote:
> > I think you can remove this completely and pretend it's true everywhere
where
> it
> > currently is (and accept the flag but make it a no-op)
> 
> Does it matter much? we'll have to be back one more time in any case.

Doesn't matter too much but at the moment (after the patch) it'll be possible to
turn off the feature with a flag, so if we roll this version in someone could
conceivably turn off the feature somewhere, and then when we roll once more with
the flag gone completely they'd break. If the flag has no effect at all after
this patch here, then this can't happen. It's a super unlikely failure case
though.

[sof, 2017-03-02 14:38:09]

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

[commit-bot: I haz the power, 2017-03-02 14:38:29]

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

[sof, 2017-03-02 14:40:08]

On 2017/03/02 14:26:50, Nico wrote:
>
https://codereview.chromium.org/2730673003/diff/1/tools/clang/blink_gc_plugin...
> File tools/clang/blink_gc_plugin/BlinkGCPluginOptions.h (right):
> 
>
https://codereview.chromium.org/2730673003/diff/1/tools/clang/blink_gc_plugin...
> tools/clang/blink_gc_plugin/BlinkGCPluginOptions.h:17: bool
> warn_stack_allocated_trace_method = true;
> On 2017/03/02 14:23:21, sof wrote:
> > On 2017/03/02 14:18:21, Nico wrote:
> > > I think you can remove this completely and pretend it's true everywhere
> where
> > it
> > > currently is (and accept the flag but make it a no-op)
> > 
> > Does it matter much? we'll have to be back one more time in any case.
> 
> Doesn't matter too much but at the moment (after the patch) it'll be possible
to
> turn off the feature with a flag, so if we roll this version in someone could
> conceivably turn off the feature somewhere, and then when we roll once more
with
> the flag gone completely they'd break. If the flag has no effect at all after
> this patch here, then this can't happen. It's a super unlikely failure case
> though.

I don't think there's an observable difference, but in the spirit of "don't put
off removing dead code", let's remove the declaration and use now.

The tests/stack_allocated.flags usage of the option verifies that it is still
recognized.

[haraken, 2017-03-02 14:43:42]

LGTM

[commit-bot: I haz the power, 2017-03-02 14:47:14]

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

[commit-bot: I haz the power, 2017-03-02 14:47:15]

Dry run: This issue passed the CQ dry run.

[Nico, 2017-03-02 14:48:46]

lgtm++

[sof, 2017-03-02 18:03:32]

Description was changed from

==========
blink_gc_plugin: always enable warn_stack_allocated_trace_method check.

R=
BUG=689864
==========

to

==========
blink_gc_plugin: always enable warn_stack_allocated_trace_method check.

R=haraken,thakis
BUG=689864
==========

[sof, 2017-03-02 18:03:42]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2017-03-02 18:04:05]

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-03-02 18:11:00]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1488477822056590,
"parent_rev": "b140095b5c1280913696fd1b98070171b8109de1", "commit_rev":
"76004798b3237c002bd84d5bfb6ff524d9ebb248"}

[commit-bot: I haz the power, 2017-03-02 18:12:18]

Description was changed from

==========
blink_gc_plugin: always enable warn_stack_allocated_trace_method check.

R=haraken,thakis
BUG=689864
==========

to

==========
blink_gc_plugin: always enable warn_stack_allocated_trace_method check.

R=haraken,thakis
BUG=689864

Review-Url: https://codereview.chromium.org/2730673003
Cr-Commit-Position: refs/heads/master@{#454307}
Committed:
https://chromium.googlesource.com/chromium/src/+/76004798b3237c002bd84d5bfb6f...
==========

[commit-bot: I haz the power, 2017-03-02 18:12:19]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/76004798b3237c002bd84d5bfb6f...