From 6d250412f131ccc77448d43de810fd5e6932cdef Mon Sep 17 00:00:00 2001 From: "yangguo@chromium.org" Date: Wed, 23 Apr 2014 15:08:03 +0000 Subject: [PATCH] Clean up some uses of Failures and MaybeObjects. R=mstarzinger@chromium.org Review URL: https://codereview.chromium.org/245963007 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20914 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/accessors.cc | 70 ++++++++++++++++++++--------------------- src/accessors.h | 60 +++++++++++++++++------------------ src/execution.cc | 2 +- src/heap.cc | 2 +- src/ia32/macro-assembler-ia32.h | 7 ----- src/objects-inl.h | 15 --------- src/objects.h | 18 ++--------- src/spaces.cc | 12 +++---- src/spaces.h | 17 +++++----- src/stub-cache.h | 1 - src/v8globals.h | 4 +-- src/x64/macro-assembler-x64.h | 7 ----- test/cctest/test-alloc.cc | 4 +-- test/cctest/test-heap.cc | 1 - 14 files changed, 88 insertions(+), 132 deletions(-) diff --git a/src/accessors.cc b/src/accessors.cc index 7a9d725..96d3d35 100644 --- a/src/accessors.cc +++ b/src/accessors.cc @@ -73,10 +73,10 @@ static C* FindInstanceOf(Isolate* isolate, Object* obj) { // Entry point that never should be called. -MaybeObject* Accessors::IllegalSetter(Isolate* isolate, - JSObject*, - Object*, - void*) { +Object* Accessors::IllegalSetter(Isolate* isolate, + JSObject*, + Object*, + void*) { UNREACHABLE(); return NULL; } @@ -90,10 +90,10 @@ Object* Accessors::IllegalGetAccessor(Isolate* isolate, } -MaybeObject* Accessors::ReadOnlySetAccessor(Isolate* isolate, - JSObject*, - Object* value, - void*) { +Object* Accessors::ReadOnlySetAccessor(Isolate* isolate, + JSObject*, + Object* value, + void*) { // According to ECMA-262, section 8.6.2.2, page 28, setting // read-only properties must be silently ignored. return value; @@ -174,9 +174,9 @@ bool Accessors::IsJSObjectFieldAccessor(Handle type, // -MaybeObject* Accessors::ArrayGetLength(Isolate* isolate, - Object* object, - void*) { +Object* Accessors::ArrayGetLength(Isolate* isolate, + Object* object, + void*) { // Traverse the prototype chain until we reach an array. JSArray* holder = FindInstanceOf(isolate, object); return holder == NULL ? Smi::FromInt(0) : holder->length(); @@ -199,10 +199,10 @@ Handle Accessors::FlattenNumber(Isolate* isolate, } -MaybeObject* Accessors::ArraySetLength(Isolate* isolate, - JSObject* object_raw, - Object* value_raw, - void*) { +Object* Accessors::ArraySetLength(Isolate* isolate, + JSObject* object_raw, + Object* value_raw, + void*) { HandleScope scope(isolate); Handle object(object_raw, isolate); Handle value(value_raw, isolate); @@ -851,18 +851,18 @@ Handle Accessors::FunctionSetPrototype(Handle function, } -MaybeObject* Accessors::FunctionGetPrototype(Isolate* isolate, - Object* object, - void*) { +Object* Accessors::FunctionGetPrototype(Isolate* isolate, + Object* object, + void*) { HandleScope scope(isolate); return *GetFunctionPrototype(isolate, Handle(object, isolate)); } -MaybeObject* Accessors::FunctionSetPrototype(Isolate* isolate, - JSObject* object, - Object* value, - void*) { +Object* Accessors::FunctionSetPrototype(Isolate* isolate, + JSObject* object, + Object* value, + void*) { Handle result; ASSIGN_RETURN_FAILURE_ON_EXCEPTION( isolate, result, @@ -885,9 +885,9 @@ const AccessorDescriptor Accessors::FunctionPrototype = { // -MaybeObject* Accessors::FunctionGetLength(Isolate* isolate, - Object* object, - void*) { +Object* Accessors::FunctionGetLength(Isolate* isolate, + Object* object, + void*) { JSFunction* function = FindInstanceOf(isolate, object); if (function == NULL) return Smi::FromInt(0); // Check if already compiled. @@ -917,9 +917,9 @@ const AccessorDescriptor Accessors::FunctionLength = { // -MaybeObject* Accessors::FunctionGetName(Isolate* isolate, - Object* object, - void*) { +Object* Accessors::FunctionGetName(Isolate* isolate, + Object* object, + void*) { JSFunction* holder = FindInstanceOf(isolate, object); return holder == NULL ? isolate->heap()->undefined_value() @@ -948,7 +948,7 @@ Handle Accessors::FunctionGetArguments(Handle function) { } -static MaybeObject* ConstructArgumentsObjectForInlinedFunction( +static Object* ConstructArgumentsObjectForInlinedFunction( JavaScriptFrame* frame, Handle inlined_function, int inlined_frame_index) { @@ -976,9 +976,9 @@ static MaybeObject* ConstructArgumentsObjectForInlinedFunction( } -MaybeObject* Accessors::FunctionGetArguments(Isolate* isolate, - Object* object, - void*) { +Object* Accessors::FunctionGetArguments(Isolate* isolate, + Object* object, + void*) { HandleScope scope(isolate); JSFunction* holder = FindInstanceOf(isolate, object); if (holder == NULL) return isolate->heap()->undefined_value(); @@ -1100,9 +1100,9 @@ class FrameFunctionIterator { }; -MaybeObject* Accessors::FunctionGetCaller(Isolate* isolate, - Object* object, - void*) { +Object* Accessors::FunctionGetCaller(Isolate* isolate, + Object* object, + void*) { HandleScope scope(isolate); DisallowHeapAllocation no_allocation; JSFunction* holder = FindInstanceOf(isolate, object); diff --git a/src/accessors.h b/src/accessors.h index 196bce5..a4452b7 100644 --- a/src/accessors.h +++ b/src/accessors.h @@ -115,40 +115,40 @@ class Accessors : public AllStatic { private: // Accessor functions only used through the descriptor. - static MaybeObject* FunctionSetPrototype(Isolate* isolate, - JSObject* object, - Object*, - void*); - static MaybeObject* FunctionGetPrototype(Isolate* isolate, - Object* object, - void*); - static MaybeObject* FunctionGetLength(Isolate* isolate, - Object* object, - void*); - static MaybeObject* FunctionGetName(Isolate* isolate, Object* object, void*); - static MaybeObject* FunctionGetArguments(Isolate* isolate, - Object* object, - void*); - static MaybeObject* FunctionGetCaller(Isolate* isolate, - Object* object, - void*); - static MaybeObject* ArraySetLength(Isolate* isolate, - JSObject* object, - Object*, - void*); - static MaybeObject* ArrayGetLength(Isolate* isolate, Object* object, void*); + static Object* FunctionSetPrototype(Isolate* isolate, + JSObject* object, + Object*, + void*); + static Object* FunctionGetPrototype(Isolate* isolate, + Object* object, + void*); + static Object* FunctionGetLength(Isolate* isolate, + Object* object, + void*); + static Object* FunctionGetName(Isolate* isolate, Object* object, void*); + static Object* FunctionGetArguments(Isolate* isolate, + Object* object, + void*); + static Object* FunctionGetCaller(Isolate* isolate, + Object* object, + void*); + static Object* ArraySetLength(Isolate* isolate, + JSObject* object, + Object*, + void*); + static Object* ArrayGetLength(Isolate* isolate, Object* object, void*); // Helper functions. static Handle FlattenNumber(Isolate* isolate, Handle value); - static MaybeObject* IllegalSetter(Isolate* isolate, - JSObject*, - Object*, - void*); + static Object* IllegalSetter(Isolate* isolate, + JSObject*, + Object*, + void*); static Object* IllegalGetAccessor(Isolate* isolate, Object* object, void*); - static MaybeObject* ReadOnlySetAccessor(Isolate* isolate, - JSObject*, - Object* value, - void*); + static Object* ReadOnlySetAccessor(Isolate* isolate, + JSObject*, + Object* value, + void*); }; } } // namespace v8::internal diff --git a/src/execution.cc b/src/execution.cc index d52d471..f88e619 100644 --- a/src/execution.cc +++ b/src/execution.cc @@ -114,7 +114,7 @@ MUST_USE_RESULT static MaybeHandle Invoke( isolate->clear_pending_message(); } - return Handle(value->ToObjectUnchecked(), isolate); + return Handle(value, isolate); } diff --git a/src/heap.cc b/src/heap.cc index 5713537..e16da7c 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -4531,7 +4531,7 @@ STRUCT_LIST(MAKE_CASE) #undef MAKE_CASE default: UNREACHABLE(); - return Failure::InternalError(); + return exception(); } int size = map->instance_size(); AllocationSpace space = SelectSpace(size, OLD_POINTER_SPACE, TENURED); diff --git a/src/ia32/macro-assembler-ia32.h b/src/ia32/macro-assembler-ia32.h index 698c81f..37a9636 100644 --- a/src/ia32/macro-assembler-ia32.h +++ b/src/ia32/macro-assembler-ia32.h @@ -1012,13 +1012,6 @@ class MacroAssembler: public Assembler { Register scratch, AllocationFlags flags); - // Helper for PopHandleScope. Allowed to perform a GC and returns - // NULL if gc_allowed. Does not perform a GC if !gc_allowed, and - // possibly returns a failure object indicating an allocation failure. - MUST_USE_RESULT MaybeObject* PopHandleScopeHelper(Register saved, - Register scratch, - bool gc_allowed); - // Helper for implementing JumpIfNotInNewSpace and JumpIfInNewSpace. void InNewSpace(Register object, Register scratch, diff --git a/src/objects-inl.h b/src/objects-inl.h index 68bd159..0ad3fa7 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -661,11 +661,6 @@ bool MaybeObject::IsRetryAfterGC() { } -bool MaybeObject::IsException() { - return this == Failure::Exception(); -} - - Failure* Failure::cast(MaybeObject* obj) { ASSERT(HAS_FAILURE_TAG(obj)); return reinterpret_cast(obj); @@ -1297,16 +1292,6 @@ AllocationSpace Failure::allocation_space() const { } -Failure* Failure::InternalError() { - return Construct(INTERNAL_ERROR); -} - - -Failure* Failure::Exception() { - return Construct(EXCEPTION); -} - - intptr_t Failure::value() const { return static_cast( reinterpret_cast(this) >> kFailureTagSize); diff --git a/src/objects.h b/src/objects.h index 71eeaa2..c0836ad 100644 --- a/src/objects.h +++ b/src/objects.h @@ -929,7 +929,6 @@ class MaybeObject BASE_EMBEDDED { public: inline bool IsFailure(); inline bool IsRetryAfterGC(); - inline bool IsException(); inline bool ToObject(Object** obj) { if (IsFailure()) return false; *obj = reinterpret_cast(this); @@ -1665,9 +1664,8 @@ class Smi: public Object { }; -// Failure is used for reporting out of memory situations and -// propagating exceptions through the runtime system. Failure objects -// are transient and cannot occur as part of the object graph. +// Failure is mainly used for reporting a situation requiring a GC. +// Failure objects are transient and cannot occur as part of the object graph. // // Failures are a single word, encoded as follows: // +-------------------------+---+--+--+ @@ -1679,9 +1677,6 @@ class Smi: public Object { // The low two bits, 0-1, are the failure tag, 11. The next two bits, // 2-3, are a failure type tag 'tt' with possible values: // 00 RETRY_AFTER_GC -// 01 EXCEPTION -// 10 INTERNAL_ERROR -// 11 OUT_OF_MEMORY_EXCEPTION // // The next three bits, 4-6, are an allocation space tag 'sss'. The // allocation space tag is 000 for all failure types except @@ -1694,13 +1689,8 @@ const int kFailureTypeTagMask = (1 << kFailureTypeTagSize) - 1; class Failure: public MaybeObject { public: - // RuntimeStubs assumes EXCEPTION = 1 in the compiler-generated code. enum Type { - RETRY_AFTER_GC = 0, - EXCEPTION = 1, // Returning this marker tells the real exception - // is in Isolate::pending_exception. - INTERNAL_ERROR = 2, - OUT_OF_MEMORY_EXCEPTION = 3 + RETRY_AFTER_GC = 0 }; inline Type type() const; @@ -1710,8 +1700,6 @@ class Failure: public MaybeObject { static inline Failure* RetryAfterGC(AllocationSpace space); static inline Failure* RetryAfterGC(); // NEW_SPACE - static inline Failure* Exception(); - static inline Failure* InternalError(); // Casting. static inline Failure* cast(MaybeObject* object); diff --git a/src/spaces.cc b/src/spaces.cc index 996afe4..5a7bc4b 100644 --- a/src/spaces.cc +++ b/src/spaces.cc @@ -1005,11 +1005,11 @@ size_t PagedSpace::CommittedPhysicalMemory() { } -MaybeObject* PagedSpace::FindObject(Address addr) { +Object* PagedSpace::FindObject(Address addr) { // Note: this function can only be called on precisely swept spaces. ASSERT(!heap()->mark_compact_collector()->in_use()); - if (!Contains(addr)) return Failure::Exception(); + if (!Contains(addr)) return Smi::FromInt(0); // Signaling not found. Page* p = Page::FromAddress(addr); HeapObjectIterator it(p, NULL); @@ -1020,7 +1020,7 @@ MaybeObject* PagedSpace::FindObject(Address addr) { } UNREACHABLE(); - return Failure::Exception(); + return Smi::FromInt(0); } @@ -3015,12 +3015,12 @@ size_t LargeObjectSpace::CommittedPhysicalMemory() { // GC support -MaybeObject* LargeObjectSpace::FindObject(Address a) { +Object* LargeObjectSpace::FindObject(Address a) { LargePage* page = FindPage(a); if (page != NULL) { return page->GetObject(); } - return Failure::Exception(); + return Smi::FromInt(0); // Signaling not found. } @@ -3101,7 +3101,7 @@ bool LargeObjectSpace::Contains(HeapObject* object) { bool owned = (chunk->owner() == this); - SLOW_ASSERT(!owned || !FindObject(address)->IsFailure()); + SLOW_ASSERT(!owned || FindObject(address)->IsHeapObject()); return owned; } diff --git a/src/spaces.h b/src/spaces.h index 1fb8562..5670f6a 100644 --- a/src/spaces.h +++ b/src/spaces.h @@ -1722,10 +1722,10 @@ class PagedSpace : public Space { bool Contains(HeapObject* o) { return Contains(o->address()); } // Given an address occupied by a live object, return that object if it is - // in this space, or Failure::Exception() if it is not. The implementation - // iterates over objects in the page containing the address, the cost is - // linear in the number of objects in the page. It may be slow. - MUST_USE_RESULT MaybeObject* FindObject(Address addr); + // in this space, or a Smi if it is not. The implementation iterates over + // objects in the page containing the address, the cost is linear in the + // number of objects in the page. It may be slow. + Object* FindObject(Address addr); // During boot the free_space_map is created, and afterwards we may need // to write it into the free list nodes that were already created. @@ -2841,10 +2841,9 @@ class LargeObjectSpace : public Space { return page_count_; } - // Finds an object for a given address, returns Failure::Exception() - // if it is not found. The function iterates through all objects in this - // space, may be slow. - MaybeObject* FindObject(Address a); + // Finds an object for a given address, returns a Smi if it is not found. + // The function iterates through all objects in this space, may be slow. + Object* FindObject(Address a); // Finds a large object page containing the given address, returns NULL // if such a page doesn't exist. @@ -2872,7 +2871,7 @@ class LargeObjectSpace : public Space { #endif // Checks whether an address is in the object area in this space. It // iterates all objects in the space. May be slow. - bool SlowContains(Address addr) { return !FindObject(addr)->IsFailure(); } + bool SlowContains(Address addr) { return FindObject(addr)->IsHeapObject(); } private: intptr_t max_capacity_; diff --git a/src/stub-cache.h b/src/stub-cache.h index 1e66613..8583d71 100644 --- a/src/stub-cache.h +++ b/src/stub-cache.h @@ -432,7 +432,6 @@ class StubCompiler BASE_EMBEDDED { Isolate* isolate_; const ExtraICState extra_ic_state_; MacroAssembler masm_; - Failure* failure_; }; diff --git a/src/v8globals.h b/src/v8globals.h index 8a89a93..67839fc 100644 --- a/src/v8globals.h +++ b/src/v8globals.h @@ -349,8 +349,8 @@ union IeeeDoubleBigEndianArchType { // AccessorCallback struct AccessorDescriptor { - MaybeObject* (*getter)(Isolate* isolate, Object* object, void* data); - MaybeObject* (*setter)( + Object* (*getter)(Isolate* isolate, Object* object, void* data); + Object* (*setter)( Isolate* isolate, JSObject* object, Object* value, void* data); void* data; }; diff --git a/src/x64/macro-assembler-x64.h b/src/x64/macro-assembler-x64.h index a216364..9dab85f 100644 --- a/src/x64/macro-assembler-x64.h +++ b/src/x64/macro-assembler-x64.h @@ -1506,13 +1506,6 @@ class MacroAssembler: public Assembler { Register scratch, AllocationFlags flags); - // Helper for PopHandleScope. Allowed to perform a GC and returns - // NULL if gc_allowed. Does not perform a GC if !gc_allowed, and - // possibly returns a failure object indicating an allocation failure. - Object* PopHandleScopeHelper(Register saved, - Register scratch, - bool gc_allowed); - // Helper for implementing JumpIfNotInNewSpace and JumpIfInNewSpace. void InNewSpace(Register object, Register scratch, diff --git a/test/cctest/test-alloc.cc b/test/cctest/test-alloc.cc index c1b1cbd..cf0b70c 100644 --- a/test/cctest/test-alloc.cc +++ b/test/cctest/test-alloc.cc @@ -104,8 +104,8 @@ TEST(StressHandles) { } -static MaybeObject* TestAccessorGet(Isolate* isolate, Object* object, void*) { - return AllocateAfterFailures(); +static Object* TestAccessorGet(Isolate* isolate, Object* object, void*) { + return *Test(); } diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc index c0cc7e6..c790b3d 100644 --- a/test/cctest/test-heap.cc +++ b/test/cctest/test-heap.cc @@ -240,7 +240,6 @@ TEST(Tagging) { Failure::RetryAfterGC(NEW_SPACE)->allocation_space()); CHECK_EQ(OLD_POINTER_SPACE, Failure::RetryAfterGC(OLD_POINTER_SPACE)->allocation_space()); - CHECK(Failure::Exception()->IsFailure()); CHECK(Smi::FromInt(Smi::kMinValue)->IsSmi()); CHECK(Smi::FromInt(Smi::kMaxValue)->IsSmi()); } -- 2.7.4