Have you tried compiling this with V8 in a shared library? The use of internals
is not allowed with using shared libraries.

Maybe we should create a wrapper somewhere in the V8 API for the parts of the
internal API used for testing in the include/v8-testing.h (or add
include/v8-internal.h with the required wrappers) so we can get d8 fully
functional when building with shared libraries.


http://codereview.chromium.org/7318002/diff/1001/src/d8.cc
File src/d8.cc (right):

http://codereview.chromium.org/7318002/diff/1001/src/d8.cc#newcode676
src/d8.cc:676: context_mutex_->Lock();
On 2011/07/07 16:04:49, yangguo wrote:
mark critical section because creating a context is not thread-safe

Isn't this already handled by the use of lockers?

http://codereview.chromium.org/7318002/diff/1001/src/d8.h
File src/d8.h (right):

http://codereview.chromium.org/7318002/diff/1001/src/d8.h#newcode117
src/d8.h:117: SourceGroup() :
Formatting, should be ':' on new line with 4 space indent, see
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Constructor_Initializer_Lists#Constructor_Initializer_Lists.

http://codereview.chromium.org/7318002/diff/1001/src/d8.h#newcode118
src/d8.h:118: #if !(defined(USING_V8_SHARED) || defined(V8_SHARED))
I think we need a comment somewhere on why thid #if stuff is required.
Maybe have a global

#if !(defined(USING_V8_SHARED) || defined(V8_SHARED))
#define CAN_USE_INTERNAL_API 1
#else
#define CAN_USE_INTERNAL_API 0
#endif  // !(defined(USING_V8_SHARED) || defined(V8_SHARED))

And use that afterwards.

http://codereview.chromium.org/7318002/diff/1001/src/d8.h#newcode122
src/d8.h:122: #endif  // USING_V8_SHARED
#endif comment not matching #if condition.

http://codereview.chromium.org/7318002/diff/1001/src/d8.h#newcode139
src/d8.h:139: #endif  // USING_V8_SHARED
Ditto.

http://codereview.chromium.org/7318002/diff/1001/src/d8.h#newcode165
src/d8.h:165: #endif  // USING_V8_SHARED
Ditto.

http://codereview.chromium.org/7318002/diff/1001/src/d8.h#newcode175
src/d8.h:175: ShellOptions() :
Code style (seee
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Constructor_Initializer_Lists#Constructor_Initializer_Lists)

http://codereview.chromium.org/7318002/diff/1001/src/d8.h#newcode178
src/d8.h:178: FLAG_stress_opt(false),
Now that this is in a class I think we should drop the FLAG_ prefix.

http://codereview.chromium.org/7318002/diff/1001/src/d8.h#newcode305
src/d8.h:305: static CounterCollection* counters_;
Should these accesses to internals be protected by #if/#endif as well?

http://codereview.chromium.org/7318002/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to