Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(192)

Issue 472923002: Add logic for catching unsafe scoped_refptr conversions to T* in returns (Closed)

Created:
6 years, 4 months ago by dcheng
Modified:
6 years, 4 months ago
Reviewers:
Ryan Sleevi, mdempsky
CC:
chromium-reviews
Project:
chromium
Visibility:
Public.

Description

Add logic for catching unsafe scoped_refptr conversions to T* in returns The tool doesn't attempt to handle all potential unsafe conversions. It only attempts to catch a conservative subset of the conversions that are definitely unsafe, such as returning a local scoped_refptr<T> or temporary. If it encounters something that isn't simple to resolve as safe/unsafe (for example, a reference may be bound to a variable with local storage, which is unsafe, or it may be bound to a member, which is probably safe), it prefers to skip rewriting anything at all, so it can be manually resolved. BUG=110610 R=rsleevi@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=290225

Patch Set 1 #

Patch Set 2 : Cleanup #

Total comments: 4

Patch Set 3 : More comments #

Messages

Total messages: 6 (0 generated)
dcheng
Just need to fix CHECK_EQ/CHECK_NE/ASSERT_EQ/ASSERT_NE/EXPECT_EQ/EXPECT_NE handling now... Yay macros.
6 years, 4 months ago (2014-08-14 17:41:34 UTC) #1
dcheng
+mdempsky who I accidentally dropped from the reviewers line.
6 years, 4 months ago (2014-08-14 20:29:02 UTC) #2
Ryan Sleevi
Seems like you may need a test for the "matches that which it shouldn't but ...
6 years, 4 months ago (2014-08-14 21:26:40 UTC) #3
dcheng
On 2014/08/14 at 21:26:40, rsleevi wrote: > Seems like you may need a test for ...
6 years, 4 months ago (2014-08-14 21:40:12 UTC) #4
Ryan Sleevi
lgtm
6 years, 4 months ago (2014-08-18 06:03:32 UTC) #5
dcheng
6 years, 4 months ago (2014-08-18 08:21:00 UTC) #6
Message was sent while issue was closed.
Committed patchset #3 manually as 290225 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698