Skip to content

Commit

Permalink
v8: fix debug builds on Windows
Browse files Browse the repository at this point in the history
Adds missing return which fixes debug builds on Windows

Fixes: #13392
Ref: https://codereview.chromium.org/2929993003/
PR-URL: #13634
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
  • Loading branch information
bzoz authored and addaleax committed Jun 12, 2017
1 parent 50e1f93 commit a0f8faa
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion deps/v8/src/api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,10 @@ void V8::SetSnapshotDataBlob(StartupData* snapshot_blob) {
i::V8::SetSnapshotBlob(snapshot_blob);
}

void* v8::ArrayBuffer::Allocator::Reserve(size_t length) { UNIMPLEMENTED(); }
void* v8::ArrayBuffer::Allocator::Reserve(size_t length) {
UNIMPLEMENTED();
return nullptr;
}

void v8::ArrayBuffer::Allocator::Free(void* data, size_t length,
AllocationMode mode) {
Expand Down

4 comments on commit a0f8faa

@MylesBorins
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch did not bump the V8 patch number. We have also generally been using the "deps" subsystem for V8.

@MylesBorins
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also... is this something backported from upstream? If not did we send a patch?

Since it appears to be backported it likely should have had the original CL meta data. Looking at the upstream issue it appears to have been reverted, is this something we need to worry about?

Sorry about being so pedantic here, just auditing some V8 stuff during the release and these commits seemed odd

@addaleax
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also... is this something backported from upstream? If not did we send a patch?

Yes, but the PR was opened before the upstream patch was merged, so it was reviewed independently as well.

Looking at the upstream issue it appears to have been reverted, is this something we need to worry about?

No. :) The patch was reverted because V8 tweaked its compiler flags later so that it became unnecessary.

@addaleax
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, and: This doesn’t need to be backported.

Please sign in to comment.