GC plugin: improve error reporting when tracing illegal fields.

GC plugin: improve error reporting when tracing illegal fields.

Add detection of trace() calls over smart pointer types that either do not
wrap up references to heap objects, or are otherwise not meant to be traced
over. In particular, CrossThread(Weak)Persistent<T> fields are now detected
as being illegal to trace over. Also consider OwnPtr<T>, RefPtr<T> and
std::unique_ptr<T> as illegal to trace over & emit a more concise error
messages for these.

R=
BUG=619149
Committed: https://crrev.com/3ba6089cd6a901b62ff5a0d8f08a2bd818edcbe8
Committed: https://crrev.com/163954bcdf26d970e2166ec2dabadff3c71fd1c8
Cr-Original-Commit-Position: refs/heads/master@{#399861}
Cr-Commit-Position: refs/heads/master@{#400653}

[sof, 2016-06-10 20:41:06]

Description was changed from

==========
Catch & report illegal tracing of fields.

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

to

==========
GC plugin: improve error reporting when tracing illegal fields.

Add detection of trace() calls over smart pointer types that either do not
wrap up references to heap objects, or are otherwise not meant to be traced
over. In particular, CrossThread(Weak)Persistent<T> fields are now detected
as being illegal to trace over. Also consider OwnPtr<T>, RefPtr<T> and
std::unique_ptr<T> as illegal to trace over & emit a more concise error
messages for these.

R=
BUG=619149
==========

[sof, 2016-06-10 20:44:32]

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

[sof, 2016-06-10 20:44:33]

please take a look (no rush.)

[haraken, 2016-06-11 05:28:42]

LGTM but we should chat with Nico about how/when to land this (in order not to
break anything).

[sof, 2016-06-11 05:40:19]

On 2016/06/11 05:28:42, haraken wrote:
> LGTM but we should chat with Nico about how/when to land this (in order not to
> break anything).

Certainly, ToT (as of a day or two ago, at least) doesn't have any of these
problematic trace() calls, but I'll continue running with this updated plugin
and address whatever comes up. I don't expect there'll be many issues to attend
to.

[sof, 2016-06-12 07:04:50]

On 2016/06/11 05:40:19, sof wrote:
> On 2016/06/11 05:28:42, haraken wrote:
> > LGTM but we should chat with Nico about how/when to land this (in order not
to
> > break anything).
> 
> Certainly, ToT (as of a day or two ago, at least) doesn't have any of these
> problematic trace() calls, but I'll continue running with this updated plugin
> and address whatever comes up. I don't expect there'll be many issues to
attend
> to.

With something like https://codereview.chromium.org/2060683002/ in place, this
CL would only improve error messages & the above concern should fall away.

[sof, 2016-06-14 06:54:44]

On 2016/06/12 07:04:50, sof wrote:
> On 2016/06/11 05:40:19, sof wrote:
> > On 2016/06/11 05:28:42, haraken wrote:
> > > LGTM but we should chat with Nico about how/when to land this (in order
not
> to
> > > break anything).
> > 
> > Certainly, ToT (as of a day or two ago, at least) doesn't have any of these
> > problematic trace() calls, but I'll continue running with this updated
plugin
> > and address whatever comes up. I don't expect there'll be many issues to
> attend
> > to.
> 
> With something like https://codereview.chromium.org/2060683002/ in place, this
> CL would only improve error messages & the above concern should fall away.

Landed as r399423, so the changes here improve error reporting only.

[Nico, 2016-06-14 11:35:15]

lgtm

https://codereview.chromium.org/2060553002/diff/1/tools/clang/blink_gc_plugin...
File tools/clang/blink_gc_plugin/RecordInfo.cpp (right):

https://codereview.chromium.org/2060553002/diff/1/tools/clang/blink_gc_plugin...
tools/clang/blink_gc_plugin/RecordInfo.cpp:606: if (!ns || ns->getName() !=
"std")
(this isn't new in this patch, but in general i think it's nicer to look up
global ::std and then compare the decl, instead of doing this with string
matching. in this case, I think Sema has a public getStdNamespace() method; also
in that other class that compares against "unique_ptr" etc)

[sof, 2016-06-14 21:01:22]

https://codereview.chromium.org/2060553002/diff/1/tools/clang/blink_gc_plugin...
File tools/clang/blink_gc_plugin/RecordInfo.cpp (right):

https://codereview.chromium.org/2060553002/diff/1/tools/clang/blink_gc_plugin...
tools/clang/blink_gc_plugin/RecordInfo.cpp:606: if (!ns || ns->getName() !=
"std")
On 2016/06/14 11:35:15, Nico (traveling...slow) wrote:
> (this isn't new in this patch, but in general i think it's nicer to look up
> global ::std and then compare the decl, instead of doing this with string
> matching. in this case, I think Sema has a public getStdNamespace() method;
also
> in that other class that compares against "unique_ptr" etc)

Thanks for the Sema tip, adjusted to use instead.

[sof, 2016-06-15 08:32:00]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2016-06-15 08:32:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/2060553002/#ps20001
(title: "tidy namespace checking")

[commit-bot: I haz the power, 2016-06-15 08:32:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2060553002/20001

[commit-bot: I haz the power, 2016-06-15 08:36:00]

Description was changed from

==========
GC plugin: improve error reporting when tracing illegal fields.

Add detection of trace() calls over smart pointer types that either do not
wrap up references to heap objects, or are otherwise not meant to be traced
over. In particular, CrossThread(Weak)Persistent<T> fields are now detected
as being illegal to trace over. Also consider OwnPtr<T>, RefPtr<T> and
std::unique_ptr<T> as illegal to trace over & emit a more concise error
messages for these.

R=
BUG=619149
==========

to

==========
GC plugin: improve error reporting when tracing illegal fields.

Add detection of trace() calls over smart pointer types that either do not
wrap up references to heap objects, or are otherwise not meant to be traced
over. In particular, CrossThread(Weak)Persistent<T> fields are now detected
as being illegal to trace over. Also consider OwnPtr<T>, RefPtr<T> and
std::unique_ptr<T> as illegal to trace over & emit a more concise error
messages for these.

R=
BUG=619149
==========

[commit-bot: I haz the power, 2016-06-15 08:36:01]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-15 08:38:19]

Description was changed from

==========
GC plugin: improve error reporting when tracing illegal fields.

Add detection of trace() calls over smart pointer types that either do not
wrap up references to heap objects, or are otherwise not meant to be traced
over. In particular, CrossThread(Weak)Persistent<T> fields are now detected
as being illegal to trace over. Also consider OwnPtr<T>, RefPtr<T> and
std::unique_ptr<T> as illegal to trace over & emit a more concise error
messages for these.

R=
BUG=619149
==========

to

==========
GC plugin: improve error reporting when tracing illegal fields.

Add detection of trace() calls over smart pointer types that either do not
wrap up references to heap objects, or are otherwise not meant to be traced
over. In particular, CrossThread(Weak)Persistent<T> fields are now detected
as being illegal to trace over. Also consider OwnPtr<T>, RefPtr<T> and
std::unique_ptr<T> as illegal to trace over & emit a more concise error
messages for these.

R=
BUG=619149

Committed: https://crrev.com/3ba6089cd6a901b62ff5a0d8f08a2bd818edcbe8
Cr-Commit-Position: refs/heads/master@{#399861}
==========

[commit-bot: I haz the power, 2016-06-15 08:38:20]

Patchset 2 (id:??) landed as
https://crrev.com/3ba6089cd6a901b62ff5a0d8f08a2bd818edcbe8
Cr-Commit-Position: refs/heads/master@{#399861}

[Nico, 2016-06-15 09:38:02]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2068983003/ by thakis@chromium.org.

The reason for reverting is: New test fails:
https://build.chromium.org/p/chromium.fyi/builders/ClangToTMac%20%28dbg%29/bu....

[sof, 2016-06-15 14:38:33]

Description was changed from

==========
GC plugin: improve error reporting when tracing illegal fields.

Add detection of trace() calls over smart pointer types that either do not
wrap up references to heap objects, or are otherwise not meant to be traced
over. In particular, CrossThread(Weak)Persistent<T> fields are now detected
as being illegal to trace over. Also consider OwnPtr<T>, RefPtr<T> and
std::unique_ptr<T> as illegal to trace over & emit a more concise error
messages for these.

R=
BUG=619149

Committed: https://crrev.com/3ba6089cd6a901b62ff5a0d8f08a2bd818edcbe8
Cr-Commit-Position: refs/heads/master@{#399861}
==========

to

==========
GC plugin: improve error reporting when tracing illegal fields.

Add detection of trace() calls over smart pointer types that either do not
wrap up references to heap objects, or are otherwise not meant to be traced
over. In particular, CrossThread(Weak)Persistent<T> fields are now detected
as being illegal to trace over. Also consider OwnPtr<T>, RefPtr<T> and
std::unique_ptr<T> as illegal to trace over & emit a more concise error
messages for these.

R=
BUG=619149

Committed: https://crrev.com/3ba6089cd6a901b62ff5a0d8f08a2bd818edcbe8
Cr-Commit-Position: refs/heads/master@{#399861}
==========

[sof, 2016-06-15 19:21:41]

Dropped back to ps#1, see https://codereview.chromium.org/2068983003/#msg9

[sof, 2016-06-20 10:50:15]

relanding

[sof, 2016-06-20 10:50:22]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2016-06-20 10:50:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/2060553002/#ps60001
(title: "robustify namespace equality checking instead")

[commit-bot: I haz the power, 2016-06-20 10:50:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2060553002/60001

[commit-bot: I haz the power, 2016-06-20 11:25:11]

Description was changed from

==========
GC plugin: improve error reporting when tracing illegal fields.

Add detection of trace() calls over smart pointer types that either do not
wrap up references to heap objects, or are otherwise not meant to be traced
over. In particular, CrossThread(Weak)Persistent<T> fields are now detected
as being illegal to trace over. Also consider OwnPtr<T>, RefPtr<T> and
std::unique_ptr<T> as illegal to trace over & emit a more concise error
messages for these.

R=
BUG=619149

Committed: https://crrev.com/3ba6089cd6a901b62ff5a0d8f08a2bd818edcbe8
Cr-Commit-Position: refs/heads/master@{#399861}
==========

to

==========
GC plugin: improve error reporting when tracing illegal fields.

Add detection of trace() calls over smart pointer types that either do not
wrap up references to heap objects, or are otherwise not meant to be traced
over. In particular, CrossThread(Weak)Persistent<T> fields are now detected
as being illegal to trace over. Also consider OwnPtr<T>, RefPtr<T> and
std::unique_ptr<T> as illegal to trace over & emit a more concise error
messages for these.

R=
BUG=619149

Committed: https://crrev.com/3ba6089cd6a901b62ff5a0d8f08a2bd818edcbe8
Cr-Commit-Position: refs/heads/master@{#399861}
==========

[commit-bot: I haz the power, 2016-06-20 11:25:13]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-20 11:27:08]

Description was changed from

==========
GC plugin: improve error reporting when tracing illegal fields.

Add detection of trace() calls over smart pointer types that either do not
wrap up references to heap objects, or are otherwise not meant to be traced
over. In particular, CrossThread(Weak)Persistent<T> fields are now detected
as being illegal to trace over. Also consider OwnPtr<T>, RefPtr<T> and
std::unique_ptr<T> as illegal to trace over & emit a more concise error
messages for these.

R=
BUG=619149

Committed: https://crrev.com/3ba6089cd6a901b62ff5a0d8f08a2bd818edcbe8
Cr-Commit-Position: refs/heads/master@{#399861}
==========

to

==========
GC plugin: improve error reporting when tracing illegal fields.

Add detection of trace() calls over smart pointer types that either do not
wrap up references to heap objects, or are otherwise not meant to be traced
over. In particular, CrossThread(Weak)Persistent<T> fields are now detected
as being illegal to trace over. Also consider OwnPtr<T>, RefPtr<T> and
std::unique_ptr<T> as illegal to trace over & emit a more concise error
messages for these.

R=
BUG=619149

Committed: https://crrev.com/3ba6089cd6a901b62ff5a0d8f08a2bd818edcbe8
Committed: https://crrev.com/163954bcdf26d970e2166ec2dabadff3c71fd1c8
Cr-Original-Commit-Position: refs/heads/master@{#399861}
Cr-Commit-Position: refs/heads/master@{#400653}
==========

[commit-bot: I haz the power, 2016-06-20 11:27:13]

Patchset 4 (id:??) landed as
https://crrev.com/163954bcdf26d970e2166ec2dabadff3c71fd1c8
Cr-Commit-Position: refs/heads/master@{#400653}