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

Issue 271113002: Revert of Prevent calls to ReturnValue::Set with pointer-valued types. (Closed)

Created:
6 years, 7 months ago by vogelheim
Modified:
6 years, 7 months ago
Reviewers:
Igor Sheludko, dcarney
CC:
v8-dev, Paweł Hajdan Jr.
Visibility:
Public.

Description

Revert of Prevent calls to ReturnValue::Set with pointer-valued types. (https://codereview.chromium.org/240013004/) Reason for revert: Looks like this broke the "V8 Linux64 ASAN" build. Original issue's description: > Prevent calls to ReturnValue::Set with pointer-valued types. > > Currently, this code will compile: > SomePointer* p = ...; > ReturnValue r = ...; > r.Set(p); > > What happens is that ReturnValue::Set has no pointer-ish overloads, but > a bool one, and hence C++ will convert the pointer to a bool and use > the Set(bool) overload. In other words, the example above is equivalent > to: r.Set(p ? true : false); Which probably isn't what the author had > in mind. This change adds a Set(void*) overload whose body forces a > compile error, to prevent this from happening inadvertently. The only > use of this indeed turned out to be an error. > > (Said error was fixed/removed in crrev.com/267393002.) > > BUG= > R=dcarney@chromium.org > > Committed: https://code.google.com/p/v8/source/detail?r=21217 R=ishell@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=21219

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -9 lines) Patch
M include/v8.h View 2 chunks +0 lines, -9 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
vogelheim
Created Revert of Prevent calls to ReturnValue::Set with pointer-valued types.
6 years, 7 months ago (2014-05-09 12:17:56 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://v8-status.appspot.com/cq/vogelheim@chromium.org/271113002/1
6 years, 7 months ago (2014-05-09 12:18:55 UTC) #2
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 7 months ago (2014-05-09 12:18:56 UTC) #3
commit-bot: I haz the power
No LGTM from a valid reviewer yet. Only full committers are accepted. Even if an ...
6 years, 7 months ago (2014-05-09 12:18:56 UTC) #4
Igor Sheludko
lgtm
6 years, 7 months ago (2014-05-09 12:25:25 UTC) #5
vogelheim
6 years, 7 months ago (2014-05-09 12:33:36 UTC) #6
Message was sent while issue was closed.
Committed patchset #1 manually as r21219 (tree was closed).

Powered by Google App Engine
This is Rietveld 408576698