LGTM with comments.
If IWYU opens a can of worms, feel free to punt on it; but since you're
messing
with includes anyway, it would be nice to do some cleanup along the way :-)
https://codereview.chromium.org/316133002/diff/80001/src/assembler.cc
File src/assembler.cc (right):
https://codereview.chromium.org/316133002/diff/80001/src/assembler.cc#newcode49
src/assembler.cc:49: #include "src/base/lazy-instance.h"
nit: alpha-sort please
https://codereview.chromium.org/316133002/diff/80001/src/base/atomicops.h
File src/base/atomicops.h (right):
https://codereview.chromium.org/316133002/diff/80001/src/base/atomicops.h#newcode134
src/base/atomicops.h:134: } } // namespace v8::bae
nit: typo
https://codereview.chromium.org/316133002/diff/80001/src/base/atomicops_internals_arm_gcc.h
File src/base/atomicops_internals_arm_gcc.h (right):
https://codereview.chromium.org/316133002/diff/80001/src/base/atomicops_internals_arm_gcc.h#newcode299
src/base/atomicops_internals_arm_gcc.h:299: } } // namespace v8::bae
nit: typo
https://codereview.chromium.org/316133002/diff/80001/src/builtins.cc
File src/builtins.cc (right):
https://codereview.chromium.org/316133002/diff/80001/src/builtins.cc#newcode1537
src/builtins.cc:1537: base::CallOnce(&once_,
&Builtins::InitBuiltinFunctionTable);
IWYU?
https://codereview.chromium.org/316133002/diff/80001/src/elements-kind.cc
File src/elements-kind.cc (right):
https://codereview.chromium.org/316133002/diff/80001/src/elements-kind.cc#newcode105
src/elements-kind.cc:105: static base::LazyInstance<ElementsKind*,
IWYU?
https://codereview.chromium.org/316133002/diff/80001/src/flags.h
File src/flags.h (right):
https://codereview.chromium.org/316133002/diff/80001/src/flags.h#newcode8
src/flags.h:8: #include "src/base/atomicops.h"
why include this here?
https://codereview.chromium.org/316133002/diff/80001/src/frames.cc
File src/frames.cc (right):
https://codereview.chromium.org/316133002/diff/80001/src/frames.cc#newcode11
src/frames.cc:11: #include "src/base/lazy-instance.h"
nit: alpha-sort please. Is this #include even used, given that no
namespace changes were required below?
https://codereview.chromium.org/316133002/diff/80001/src/heap.cc
File src/heap.cc (right):
https://codereview.chromium.org/316133002/diff/80001/src/heap.cc#newcode24
src/heap.cc:24: #include "src/base/once.h"
nit: alpha-sort please
https://codereview.chromium.org/316133002/diff/80001/src/mark-compact.cc
File src/mark-compact.cc (right):
https://codereview.chromium.org/316133002/diff/80001/src/mark-compact.cc#newcode2975
src/mark-compact.cc:2975: base::NoBarrier_CompareAndSwap(
IWYU?
https://codereview.chromium.org/316133002/diff/80001/src/objects-inl.h
File src/objects-inl.h (right):
https://codereview.chromium.org/316133002/diff/80001/src/objects-inl.h#newcode1126
src/objects-inl.h:1126: reinterpret_cast<Object*>(base::Acquire_Load( \
IWYU?
https://codereview.chromium.org/316133002/diff/80001/src/optimizing-compiler-thread.cc
File src/optimizing-compiler-thread.cc (right):
https://codereview.chromium.org/316133002/diff/80001/src/optimizing-compiler-thread.cc#newcode54
src/optimizing-compiler-thread.cc:54: switch
(static_cast<StopFlag>(base::Acquire_Load(&stop_thread_))) {
IWYU?
https://codereview.chromium.org/316133002/diff/80001/src/platform-cygwin.cc
File src/platform-cygwin.cc (right):
https://codereview.chromium.org/316133002/diff/80001/src/platform-cygwin.cc#newcode23
src/platform-cygwin.cc:23: #include "src/base/win32-headers.h"
nit: alpha-sort please
https://codereview.chromium.org/316133002/diff/80001/src/platform-posix.cc
File src/platform-posix.cc (right):
https://codereview.chromium.org/316133002/diff/80001/src/platform-posix.cc#newcode602
src/platform-posix.cc:602: static base::Atomic32
tls_base_offset_initialized = 0;
IWYU?
https://codereview.chromium.org/316133002/diff/80001/src/platform.h
File src/platform.h (right):
https://codereview.chromium.org/316133002/diff/80001/src/platform.h#newcode47
src/platform.h:47: #include "src/win32-math.h"
It would seem logical to keep win32-headers.h and win32-math.h
together...
https://codereview.chromium.org/316133002/diff/80001/src/platform/condition-variable.h
File src/platform/condition-variable.h (right):
https://codereview.chromium.org/316133002/diff/80001/src/platform/condition-variable.h#newcode109
src/platform/condition-variable.h:109: typedef base::LazyStaticInstance<
IWYU?
https://codereview.chromium.org/316133002/diff/80001/src/platform/mutex.h
File src/platform/mutex.h (right):
https://codereview.chromium.org/316133002/diff/80001/src/platform/mutex.h#newcode9
src/platform/mutex.h:9: #include "src/base/lazy-instance.h"
nit: alpha-sort please
https://codereview.chromium.org/316133002/diff/80001/src/platform/time.cc
File src/platform/time.cc (right):
https://codereview.chromium.org/316133002/diff/80001/src/platform/time.cc#newcode196
src/platform/time.cc:196: static base::LazyStaticInstance<Clock,
base::DefaultConstructTrait<Clock>,
IWYU?
https://codereview.chromium.org/316133002/diff/80001/src/runtime-profiler.h
File src/runtime-profiler.h (right):
https://codereview.chromium.org/316133002/diff/80001/src/runtime-profiler.h#newcode9
src/runtime-profiler.h:9: #include "src/base/atomicops.h"
why include this?
https://codereview.chromium.org/316133002/diff/80001/src/spaces.h
File src/spaces.h (right):
https://codereview.chromium.org/316133002/diff/80001/src/spaces.h#newcode294
src/spaces.h:294: return
reinterpret_cast<MemoryChunk*>(base::Acquire_Load(&next_chunk_));
IWYU?
https://codereview.chromium.org/316133002/diff/80001/src/store-buffer.cc
File src/store-buffer.cc (right):
https://codereview.chromium.org/316133002/diff/80001/src/store-buffer.cc#newcode348
src/store-buffer.cc:348:
base::NoBarrier_Load(reinterpret_cast<base::AtomicWord*>(slot)));
IWYU?
https://codereview.chromium.org/316133002/diff/80001/src/unbound-queue-inl.h
File src/unbound-queue-inl.h (right):
https://codereview.chromium.org/316133002/diff/80001/src/unbound-queue-inl.h#newcode10
src/unbound-queue-inl.h:10: #include "src/base/atomicops.h"
You can drop this here if you move it to unbound-queue.h
https://codereview.chromium.org/316133002/diff/80001/src/unbound-queue.h
File src/unbound-queue.h (right):
https://codereview.chromium.org/316133002/diff/80001/src/unbound-queue.h#newcode37
src/unbound-queue.h:37: base::AtomicWord divider_; // Node*
IWYU?
https://codereview.chromium.org/316133002/diff/80001/src/v8.cc
File src/v8.cc (right):
https://codereview.chromium.org/316133002/diff/80001/src/v8.cc#newcode21
src/v8.cc:21: #include "src/base/once.h"
nit: alpha-sort please
https://codereview.chromium.org/316133002/diff/80001/test/cctest/test-platform-win32.cc
File test/cctest/test-platform-win32.cc (right):
https://codereview.chromium.org/316133002/diff/80001/test/cctest/test-platform-win32.cc#newcode35
test/cctest/test-platform-win32.cc:35: #include
"src/base/win32-headers.h"
nit: alpha-sort please
https://codereview.chromium.org/316133002/diff/80001/tools/gyp/v8.gyp
File tools/gyp/v8.gyp (right):
https://codereview.chromium.org/316133002/diff/80001/tools/gyp/v8.gyp#newcode1064
tools/gyp/v8.gyp:1064: '../../src/base/atomicops_internals_x86_gcc.cc',
I think all the other atomicops_internals_* files should be listed here
too, so that GYP knows which targets to rebuild when they change (which,
granted, they don't do often...).
https://codereview.chromium.org/316133002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
