Issue 5005001: Split globals.h into two parts, where only one depends on V8.

Issue 5005001: Split globals.h into two parts, where only one depends on V8. (Closed)

Created:
10 years, 1 month ago by Lasse Reichstein

Modified:
9 years, 6 months ago

Reviewers:
Rico

CC:
v8-dev

Visibility:
Public.

Description

Split globals.h into two parts, where only one depends on V8. Made allocation.{h,cc} independent of V8, allowing utils.h to allocate vectors and collectors.

Patch Set 2 : Addressed review comments. #

Created: 10 years, 1 month ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+522 lines, -456 lines)			Patch
M	include/v8.h	View		6 chunks	+13 lines, -13 lines	0 comments	Download
M	src/allocation.h	View		1 chunk	+4 lines, -0 lines	0 comments	Download
M	src/allocation.cc	View		5 chunks	+12 lines, -6 lines	0 comments	Download
M	src/api.cc	View	1	2 chunks	+4 lines, -1 line	0 comments	Download
M	src/checks.h	View		1 chunk	+0 lines, -2 lines	0 comments	Download
M	src/globals.h	View	1	6 chunks	+8 lines, -425 lines	0 comments	Download
M	src/list.h	View		1 chunk	+0 lines, -8 lines	0 comments	Download
M	src/scanner-base.cc	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	src/utils.h	View		1 chunk	+2 lines, -0 lines	0 comments	Download
M	src/v8.h	View		1 chunk	+1 line, -1 line	0 comments	Download
A	src/v8globals.h	View	1	1 chunk	+464 lines, -0 lines	0 comments	Download
M	src/virtual-frame.h	View	1	2 chunks	+13 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Lasse Reichstein

http://codereview.chromium.org/5005001/diff/1/include/v8.h File include/v8.h (right): http://codereview.chromium.org/5005001/diff/1/include/v8.h#newcode3286 include/v8.h:3286: static const int kApiIntSize = sizeof(int); // NOLINT Renamed ...

10 years, 1 month ago (2010-11-15 12:32:55 UTC) #1

Rico

LGTM http://codereview.chromium.org/5005001/diff/1/src/allocation.cc File src/allocation.cc (right): http://codereview.chromium.org/5005001/diff/1/src/allocation.cc#newcode90 src/allocation.cc:90: memcpy(result, str, length); do we know that kCharSize ...

10 years, 1 month ago (2010-11-15 13:10:14 UTC) #2

Lasse Reichstein

10 years, 1 month ago (2010-11-15 13:21:10 UTC) #3

http://codereview.chromium.org/5005001/diff/1/src/allocation.cc
File src/allocation.cc (right):

http://codereview.chromium.org/5005001/diff/1/src/allocation.cc#newcode90
src/allocation.cc:90: memcpy(result, str, length);
Yes. kCharSize is defined as sizeof(char), and that's pretty much the only
guaranteed size in the C and C++ standards.
It may not be eight bits, but it definitely has size 1.

http://codereview.chromium.org/5005001/diff/1/src/api.cc
File src/api.cc (right):

http://codereview.chromium.org/5005001/diff/1/src/api.cc#newcode117
src/api.cc:117: 
Done.

http://codereview.chromium.org/5005001/diff/1/src/v8globals.h
File src/v8globals.h (right):

http://codereview.chromium.org/5005001/diff/1/src/v8globals.h#newcode1
src/v8globals.h:1: // Copyright 2006-2009 the V8 project authors. All rights
reserved.
Done.

http://codereview.chromium.org/5005001/diff/1/src/v8globals.h#newcode38
src/v8globals.h:38: 
It is taken verbatim from the old globals.h.

http://codereview.chromium.org/5005001/diff/1/src/virtual-frame.h
File src/virtual-frame.h (right):

http://codereview.chromium.org/5005001/diff/1/src/virtual-frame.h#newcode47
src/virtual-frame.h:47: #include "utils.h"
Done.

Expand Messages | Collapse Messages