New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 604476 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Email to this user bounced
Closed: Apr 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocked on:
issue 604993



Sign in to add a comment

Treat Blink GC plugin's warn-raw-ptr warning as an error, always.

Reported by sigbjo...@opera.com, Apr 18 2016

Issue description

The Oilpan-based Blink codebase is "raw heap pointer"-clean -- all references to heap types T are properly accounted for in class field declarations and inside collections, via Member<T> and others (Persistent, UntracedMember, WeakMember etc.)

The Blink clang GC plugin has a warning option (warn-raw-ptr=1) which turns on a warning (or error with -Werror) if it comes across a bare T* (or T&) in a class declaration. There's no need for this to remain a warning (for fear of it being too noisy), we can simply consider it an error by default and retire the warning option.
 

Comment 1 by sigbjo...@opera.com, Apr 18 2016

Labels: -Pri-3 OS-All Pri-2
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 19 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f3af836424457dc1e7310bc4aad6527f08094dd6

commit f3af836424457dc1e7310bc4aad6527f08094dd6
Author: sigbjornf <sigbjornf@opera.com>
Date: Tue Apr 19 16:41:34 2016

Always enable warn-raw-ptr's check of raw heap pointers.

This warning option has been default-enabled with Oilpan since 3a192c3
(2015-11-25), checking that we do not keep unmanaged raw pointers or
references in class field types. With the Blink codebase adhering
to that (desirable) constraint, this extra warning has been working
well to keep the codebase in that state.

Make the check always apply with no possibility of opting out; we want
it permanently on.

R=
BUG= 604476 

Review URL: https://codereview.chromium.org/1901643003

Cr-Commit-Position: refs/heads/master@{#388222}

[modify] https://crrev.com/f3af836424457dc1e7310bc4aad6527f08094dd6/tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp
[modify] https://crrev.com/f3af836424457dc1e7310bc4aad6527f08094dd6/tools/clang/blink_gc_plugin/BlinkGCPluginConsumer.cpp
[modify] https://crrev.com/f3af836424457dc1e7310bc4aad6527f08094dd6/tools/clang/blink_gc_plugin/BlinkGCPluginConsumer.h
[modify] https://crrev.com/f3af836424457dc1e7310bc4aad6527f08094dd6/tools/clang/blink_gc_plugin/BlinkGCPluginOptions.h
[modify] https://crrev.com/f3af836424457dc1e7310bc4aad6527f08094dd6/tools/clang/blink_gc_plugin/CheckFieldsVisitor.cpp
[modify] https://crrev.com/f3af836424457dc1e7310bc4aad6527f08094dd6/tools/clang/blink_gc_plugin/CheckFieldsVisitor.h
[delete] https://crrev.com/30e5f0646070b6042fb5224598e9194edf71c801/tools/clang/blink_gc_plugin/tests/raw_ptr_to_gc_managed_class.flags
[modify] https://crrev.com/f3af836424457dc1e7310bc4aad6527f08094dd6/tools/clang/blink_gc_plugin/tests/raw_ptr_to_gc_managed_class_error.flags

Comment 3 by sigbjo...@opera.com, Apr 19 2016

Status: Fixed (was: Assigned)

Comment 4 by sigbjo...@opera.com, Apr 20 2016

Blockedon: 604993
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 29 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/01d8280352825197825e139a60f38ab0cf11b9b8

commit 01d8280352825197825e139a60f38ab0cf11b9b8
Author: sigbjornf <sigbjornf@opera.com>
Date: Fri Apr 29 12:44:34 2016

blink_gc_plugin: drop no-op options.

With the updated GC clang plugin that rolled out as part of  issue 604993 ,
we can now assume that the enable-oilpan and warn-raw-ptr options are
by default always on & consequently doesn't need to be passed in by
blink_gc_plugin_flags.py

R=
BUG= 604463 , 604476 

Review-Url: https://codereview.chromium.org/1926003003
Cr-Commit-Position: refs/heads/master@{#390631}

[modify] https://crrev.com/01d8280352825197825e139a60f38ab0cf11b9b8/tools/clang/scripts/blink_gc_plugin_flags.py

Sign in to add a comment