Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2724353002:
Enable Blink GC plugin check for stack allocated classes. (Closed)
DescriptionEnable Blink GC plugin check for stack allocated classes.
The latest clang roll (crbug.com/685244) included a GC plugin with
support for checking STACK_ALLOCATED() classes w/ unwanted trace
method definitions. Add support for that check to the build system,
unconditionally enabled.
R=haraken,thakis
BUG=689874
Review-Url: https://codereview.chromium.org/2724353002
Cr-Commit-Position: refs/heads/master@{#454306}
Committed: https://chromium.googlesource.com/chromium/src/+/b140095b5c1280913696fd1b98070171b8109de1
Patch Set 1 #
Total comments: 3
Patch Set 2 : unconditionally enable the warning/check #Messages
Total messages: 22 (15 generated)
The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
sigbjornf@opera.com changed reviewers: + oilpan-reviews@chromium.org, thakis@chromium.org
please take a look. The latest clang roll appears to have settled in very well, so let's turn on this option.
LGTM
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2724353002/diff/1/third_party/WebKit/Source/B... File third_party/WebKit/Source/BUILD.gn (right): https://codereview.chromium.org/2724353002/diff/1/third_party/WebKit/Source/B... third_party/WebKit/Source/BUILD.gn:38: blink_gc_plugin_option_warn_stack_allocated_trace_methods = true if the plan is to remove this soon again… https://codereview.chromium.org/2724353002/diff/1/third_party/WebKit/Source/B... third_party/WebKit/Source/BUILD.gn:161: if (blink_gc_plugin_option_warn_stack_allocated_trace_methods) { …maybe just add it unconditionally here?
The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2724353002/diff/1/third_party/WebKit/Source/B... File third_party/WebKit/Source/BUILD.gn (right): https://codereview.chromium.org/2724353002/diff/1/third_party/WebKit/Source/B... third_party/WebKit/Source/BUILD.gn:161: if (blink_gc_plugin_option_warn_stack_allocated_trace_methods) { On 2017/03/02 14:08:56, Nico wrote: > …maybe just add it unconditionally here? Yes, selectively turning it off shouldn't be needed in practice. Done.
lgtm++
Description was changed from ========== Enable Blink GC plugin check for stack allocated classes. The latest clang roll (crbug.com/685244) included a GC plugin with support for checking STACK_ALLOCATED() classes w/ unwanted trace method definitions. Add support for that check to the build system, turning it on by default. R= BUG=689874 ========== to ========== Enable Blink GC plugin check for stack allocated classes. The latest clang roll (crbug.com/685244) included a GC plugin with support for checking STACK_ALLOCATED() classes w/ unwanted trace method definitions. Add support for that check to the build system, turning it on by default. R=haraken,thakis BUG=689874 ==========
Description was changed from ========== Enable Blink GC plugin check for stack allocated classes. The latest clang roll (crbug.com/685244) included a GC plugin with support for checking STACK_ALLOCATED() classes w/ unwanted trace method definitions. Add support for that check to the build system, turning it on by default. R=haraken,thakis BUG=689874 ========== to ========== Enable Blink GC plugin check for stack allocated classes. The latest clang roll (crbug.com/685244) included a GC plugin with support for checking STACK_ALLOCATED() classes w/ unwanted trace method definitions. Add support for that check to the build system, unconditionally enabled. R=haraken,thakis BUG=689874 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by sigbjornf@opera.com
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/2724353002/#ps20001 (title: "unconditionally enable the warning/check")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1488477747918100,
"parent_rev": "3ae2fa288ba397536af9589f7b56f20cce4dc442", "commit_rev":
"b140095b5c1280913696fd1b98070171b8109de1"}
Message was sent while issue was closed.
Description was changed from ========== Enable Blink GC plugin check for stack allocated classes. The latest clang roll (crbug.com/685244) included a GC plugin with support for checking STACK_ALLOCATED() classes w/ unwanted trace method definitions. Add support for that check to the build system, unconditionally enabled. R=haraken,thakis BUG=689874 ========== to ========== Enable Blink GC plugin check for stack allocated classes. The latest clang roll (crbug.com/685244) included a GC plugin with support for checking STACK_ALLOCATED() classes w/ unwanted trace method definitions. Add support for that check to the build system, unconditionally enabled. R=haraken,thakis BUG=689874 Review-Url: https://codereview.chromium.org/2724353002 Cr-Commit-Position: refs/heads/master@{#454306} Committed: https://chromium.googlesource.com/chromium/src/+/b140095b5c1280913696fd1b9807... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/b140095b5c1280913696fd1b9807... |
