LGTM On Mon, Sep 8, 2008 at 9:18 AM, <[EMAIL PROTECTED]> wrote: > Mads, > > I'd like you to do a code review. To review this change, run > > gvn review --project https://v8.googlecode.com/svn [EMAIL PROTECTED]/[EMAIL > PROTECTED] > > Alternatively, to review the latest snapshot of this change > branch, run > > gvn --project https://v8.googlecode.com/svn review [EMAIL > PROTECTED]/fix-broken-build > > to review the following change: > > [EMAIL PROTECTED]/[EMAIL PROTECTED] | [EMAIL PROTECTED] | 2008-09-08 08:18:02 > +-100 (Mon, 08 Sep 2008) > > Description: > > Fix broken build. Sorry about that. > > > > Affected Paths: > R //branches/bleeding_edge/include/v8.h > M //branches/bleeding_edge/src/debug.h > M //branches/bleeding_edge/src/v8threads.cc > M //branches/bleeding_edge/test/cctest/test-api.cc > > > This is a semiautomated message from "gvn mail". See > <http://code.google.com/p/gvn/> to learn more. > > Index: include/v8.h > =================================================================== > --- include/v8.h (added) > +++ include/v8.h (^/changes/[EMAIL > PROTECTED]/fix-broken-build/bleeding_edge/include/[EMAIL PROTECTED]) > @@ -2123,11 +2123,11 @@ class EXPORT Locker { > */ > static void StopPreemption(); > > -#ifdef DEBUG > - static void AssertIsLocked(); > -#else > - static inline void AssertIsLocked() { } > -#endif > + /** > + * Returns whether or not the locker is locked by the current thread. > + */ > + static bool IsLocked(); > + > private: > bool has_lock_; > bool top_level_; > Index: src/debug.h > =================================================================== > --- src/debug.h (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > +++ src/debug.h (^/changes/[EMAIL > PROTECTED]/fix-broken-build/bleeding_edge/src/[EMAIL PROTECTED]) > @@ -25,8 +25,8 @@ > // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE > // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > > -#ifndef V8_DEBUG_H_ > -#define V8_DEBUG_H_ > +#ifndef V8_V8_DEBUG_H_ > +#define V8_V8_DEBUG_H_ > > #include "../include/v8-debug.h" > #include "assembler.h" > @@ -574,4 +574,4 @@ class Debug_Address { > > } } // namespace v8::internal > > -#endif // V8_DEBUG_H_ > +#endif // V8_V8_DEBUG_H_ > Index: src/v8threads.cc > =================================================================== > --- src/v8threads.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) > +++ src/v8threads.cc (^/changes/[EMAIL > PROTECTED]/fix-broken-build/bleeding_edge/src/[EMAIL PROTECTED]) > @@ -54,11 +54,9 @@ Locker::Locker() : has_lock_(false), top_level_(tr > } > > > -#ifdef DEBUG > -void Locker::AssertIsLocked() { > - ASSERT(internal::ThreadManager::IsLockedByCurrentThread()); > +bool Locker::IsLocked() { > + return internal::ThreadManager::IsLockedByCurrentThread(); > } > -#endif > > > Locker::~Locker() { > @@ -282,7 +280,7 @@ static v8::internal::ContextSwitcher* switcher; > > > void ContextSwitcher::StartPreemption(int every_n_ms) { > - Locker::AssertIsLocked(); > + ASSERT(Locker::IsLocked()); > if (switcher == NULL) { > switcher = new ContextSwitcher(every_n_ms); > switcher->Start(); > @@ -293,7 +291,7 @@ void ContextSwitcher::StartPreemption(int every_n_ > > > void ContextSwitcher::StopPreemption() { > - Locker::AssertIsLocked(); > + ASSERT(Locker::IsLocked()); > if (switcher != NULL) { > switcher->Stop(); > delete(switcher); > @@ -312,7 +310,7 @@ void ContextSwitcher::Run() { > > > void ContextSwitcher::Stop() { > - Locker::AssertIsLocked(); > + ASSERT(Locker::IsLocked()); > keep_going_ = false; > preemption_semaphore_->Signal(); > Join(); > @@ -325,7 +323,7 @@ void ContextSwitcher::WaitForPreemption() { > > > void ContextSwitcher::PreemptionReceived() { > - Locker::AssertIsLocked(); > + ASSERT(Locker::IsLocked()); > switcher->preemption_semaphore_->Signal(); > } > > Index: test/cctest/test-api.cc > =================================================================== > --- test/cctest/test-api.cc (^/branches/bleeding_edge/test/cctest/[EMAIL > PROTECTED]) > +++ test/cctest/test-api.cc (^/changes/[EMAIL > PROTECTED]/fix-broken-build/bleeding_edge/test/cctest/[EMAIL PROTECTED]) > @@ -4563,7 +4563,7 @@ void ApiTestFuzzer::CallTest() { > > > static v8::Handle<Value> ThrowInJS(const v8::Arguments& args) { > - v8::Locker::AssertIsLocked(); > + CHECK(v8::Locker::IsLocked()); > ApiTestFuzzer::Fuzz(); > v8::Unlocker unlocker; > const char* code = "throw 7;"; > @@ -4586,7 +4586,7 @@ static v8::Handle<Value> ThrowInJS(const v8::Argum > > > static v8::Handle<Value> ThrowInJSNoCatch(const v8::Arguments& args) { > - v8::Locker::AssertIsLocked(); > + CHECK(v8::Locker::IsLocked()); > ApiTestFuzzer::Fuzz(); > v8::Unlocker unlocker; > const char* code = "throw 7;"; > @@ -4604,7 +4604,7 @@ static v8::Handle<Value> ThrowInJSNoCatch(const v8 > // as part of the locking aggregation tests. > TEST(NestedLockers) { > v8::Locker locker; > - v8::Locker::AssertIsLocked(); > + CHECK(v8::Locker::IsLocked()); > v8::HandleScope scope; > LocalContext env; > Local<v8::FunctionTemplate> fun_templ = > v8::FunctionTemplate::New(ThrowInJS); > @@ -4648,7 +4648,7 @@ THREADED_TEST(RecursiveLocking) { > v8::Locker locker; > { > v8::Locker locker2; > - v8::Locker::AssertIsLocked(); > + CHECK(v8::Locker::IsLocked()); > } > } > > >
--~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
