Reviewers: Kevin Millikin,
Description:
Refactoring only: Make the handling of PropertyType more explicit.
Do not rely on 'default' clauses or 'if's when analysing a PropertyType,
because
this makes it hard to find the relevant places when a new type is added.
Note
that the detection of "phantom property types" is left untouched, because
this
might have a performance impact, especially for the GC (to be investigated).
This is a preliminary step for introducing a new kind of map transition.
Please review this at http://codereview.chromium.org/8491016/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/ic.cc
M src/objects-inl.h
M src/objects-printer.cc
M src/objects.h
M src/objects.cc
M src/property.h
M src/runtime.cc
M src/v8globals.h
Index: src/ic.cc
diff --git a/src/ic.cc b/src/ic.cc
index
fbe77b09d23fa7daccabb0a842ae49221adf9b77..7b0a815aa646fd983b4a5341bd595911ac5a44f7
100644
--- a/src/ic.cc
+++ b/src/ic.cc
@@ -1354,7 +1354,6 @@ void StoreIC::UpdateCaches(LookupResult* lookup,
break;
case MAP_TRANSITION: {
if (lookup->GetAttributes() != NONE) return;
- ASSERT(type == MAP_TRANSITION);
Handle<Map> transition(lookup->GetTransitionMap());
int index = transition->PropertyIndexFor(*name);
code = isolate()->stub_cache()->ComputeStoreField(
@@ -1390,7 +1389,13 @@ void StoreIC::UpdateCaches(LookupResult* lookup,
code = isolate()->stub_cache()->ComputeStoreInterceptor(
name, receiver, strict_mode);
break;
- default:
+ case CONSTANT_FUNCTION:
+ case CONSTANT_TRANSITION:
+ case ELEMENTS_TRANSITION:
+ return;
+ case HANDLER:
+ case NULL_DESCRIPTOR:
+ UNREACHABLE();
return;
}
@@ -1719,6 +1724,13 @@ void KeyedStoreIC::UpdateCaches(LookupResult* lookup,
Handle<Code> code;
switch (type) {
+ case CONSTANT_FUNCTION:
+ case HANDLER:
+ case INTERCEPTOR:
+ case ELEMENTS_TRANSITION:
+ case NULL_DESCRIPTOR:
+ UNREACHABLE();
+ return;
case FIELD:
code = isolate()->stub_cache()->ComputeKeyedStoreField(
name, receiver, lookup->GetFieldIndex(),
@@ -1726,7 +1738,6 @@ void KeyedStoreIC::UpdateCaches(LookupResult* lookup,
break;
case MAP_TRANSITION:
if (lookup->GetAttributes() == NONE) {
- ASSERT(type == MAP_TRANSITION);
Handle<Map> transition(lookup->GetTransitionMap());
int index = transition->PropertyIndexFor(*name);
code = isolate()->stub_cache()->ComputeKeyedStoreField(
@@ -1734,7 +1745,9 @@ void KeyedStoreIC::UpdateCaches(LookupResult* lookup,
break;
}
// fall through.
- default:
+ case NORMAL:
+ case CALLBACKS:
+ case CONSTANT_TRANSITION:
// Always rewrite to the generic case so that we do not
// repeatedly try to rewrite.
code = (strict_mode == kStrictMode)
Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index
55a3b2ff40d247e73d1c7fdf1d1c8d7c9751d708..3b5f58ab2ecf2d61a07ea2b5fdcd0b64402a435a
100644
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -1865,9 +1865,7 @@ bool DescriptorArray::IsProperty(int
descriptor_number) {
bool DescriptorArray::IsTransition(int descriptor_number) {
- PropertyType t = GetType(descriptor_number);
- return t == MAP_TRANSITION || t == CONSTANT_TRANSITION ||
- t == ELEMENTS_TRANSITION;
+ return IsTransitionType(GetType(descriptor_number));
}
Index: src/objects-printer.cc
diff --git a/src/objects-printer.cc b/src/objects-printer.cc
index
1ca97de8675e7ebf8972b43ef2c1b3848f4defc5..c9f3f842dff455516bdf53a09da306dd876e0dca
100644
--- a/src/objects-printer.cc
+++ b/src/objects-printer.cc
@@ -295,7 +295,9 @@ void JSObject::PrintProperties(FILE* out) {
case NULL_DESCRIPTOR:
PrintF(out, "(null descriptor)\n");
break;
- default:
+ case NORMAL: // only in slow mode
+ case HANDLER: // only in lookup results, not in descriptors
+ case INTERCEPTOR: // only in lookup results, not in descriptors
UNREACHABLE();
break;
}
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
3a18184673eef0a52f5f25c4e285356678373152..bddf1a8566d64b47f397c91ab8b9da861b0fa5fe
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -3023,10 +3023,11 @@ MaybeObject*
JSObject::SetPropertyForResult(LookupResult* result,
case NULL_DESCRIPTOR:
case ELEMENTS_TRANSITION:
return ConvertDescriptorToFieldAndMapTransition(name, value,
attributes);
- default:
+ case HANDLER:
UNREACHABLE();
+ return value;
}
- UNREACHABLE();
+ UNREACHABLE(); // keep the compiler happy
return value;
}
@@ -3111,10 +3112,11 @@ MaybeObject*
JSObject::SetLocalPropertyIgnoreAttributes(
case NULL_DESCRIPTOR:
case ELEMENTS_TRANSITION:
return ConvertDescriptorToFieldAndMapTransition(name, value,
attributes);
- default:
+ case HANDLER:
UNREACHABLE();
+ return value;
}
- UNREACHABLE();
+ UNREACHABLE(); // keep the compiler happy
return value;
}
@@ -3400,8 +3402,10 @@ MaybeObject*
JSObject::NormalizeProperties(PropertyNormalizationMode mode,
case INTERCEPTOR:
case ELEMENTS_TRANSITION:
break;
- default:
+ case HANDLER:
+ case NORMAL:
UNREACHABLE();
+ break;
}
}
@@ -7007,9 +7011,7 @@ void Map::CreateOneBackPointer(Map* target) {
void Map::CreateBackPointers() {
DescriptorArray* descriptors = instance_descriptors();
for (int i = 0; i < descriptors->number_of_descriptors(); i++) {
- if (descriptors->GetType(i) == MAP_TRANSITION ||
- descriptors->GetType(i) == ELEMENTS_TRANSITION ||
- descriptors->GetType(i) == CONSTANT_TRANSITION) {
+ if (descriptors->IsTransition(i)) {
Object* object = reinterpret_cast<Object*>(descriptors->GetValue(i));
if (object->IsMap()) {
CreateOneBackPointer(reinterpret_cast<Map*>(object));
@@ -7047,9 +7049,7 @@ void Map::ClearNonLiveTransitions(Heap* heap, Object*
real_prototype) {
// map is not reached again by following a back pointer from a
// non-live object.
PropertyDetails details(Smi::cast(contents->get(i + 1)));
- if (details.type() == MAP_TRANSITION ||
- details.type() == ELEMENTS_TRANSITION ||
- details.type() == CONSTANT_TRANSITION) {
+ if (IsTransitionType(details.type())) {
Object* object = reinterpret_cast<Object*>(contents->get(i));
if (object->IsMap()) {
Map* target = reinterpret_cast<Map*>(object);
@@ -8034,7 +8034,7 @@ const char* Code::PropertyType2String(PropertyType
type) {
case CONSTANT_TRANSITION: return "CONSTANT_TRANSITION";
case NULL_DESCRIPTOR: return "NULL_DESCRIPTOR";
}
- UNREACHABLE();
+ UNREACHABLE(); // keep the compiler happy
return NULL;
}
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index
b423cd427994ec919f328da4b379fdf7b3a7c052..c54df59d97ececfd3e8ef05a6388624004350395
100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -207,8 +207,7 @@ class PropertyDetails BASE_EMBEDDED {
bool IsTransition() {
PropertyType t = type();
ASSERT(t != INTERCEPTOR);
- return t == MAP_TRANSITION || t == CONSTANT_TRANSITION ||
- t == ELEMENTS_TRANSITION;
+ return IsTransitionType(t);
}
bool IsProperty() {
Index: src/property.h
diff --git a/src/property.h b/src/property.h
index
ffea41e66688ae46af057dc51736f0bad54384c1..133abc1b42ef0bd50e307485f5e0f7cc3fb7a300
100644
--- a/src/property.h
+++ b/src/property.h
@@ -292,10 +292,10 @@ class LookupResult BASE_EMBEDDED {
}
}
+
Map* GetTransitionMap() {
ASSERT(lookup_type_ == DESCRIPTOR_TYPE);
- ASSERT(type() == MAP_TRANSITION || type() == CONSTANT_TRANSITION ||
- type() == ELEMENTS_TRANSITION);
+ ASSERT(IsTransitionType(type()));
return Map::cast(GetValue());
}
Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index
29807da9f296e3e5e4bf33a89fa329793fffac0d..c1a87dbf824f4721f0e6a64031fcc4ca922b68d3
100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -10406,10 +10406,11 @@ static MaybeObject* DebugLookupResultValue(Heap*
heap,
case CONSTANT_TRANSITION:
case NULL_DESCRIPTOR:
return heap->undefined_value();
- default:
+ case HANDLER:
UNREACHABLE();
+ return heap->undefined_value();
}
- UNREACHABLE();
+ UNREACHABLE(); // keep the compiler happy
return heap->undefined_value();
}
Index: src/v8globals.h
diff --git a/src/v8globals.h b/src/v8globals.h
index
560e36874c19102bb8a8be9343efe28d561820f1..3a29a64453aa70baf3df85f68babce4cdf51b8f3
100644
--- a/src/v8globals.h
+++ b/src/v8globals.h
@@ -29,6 +29,7 @@
#define V8_V8GLOBALS_H_
#include "globals.h"
+#include "checks.h"
namespace v8 {
namespace internal {
@@ -347,6 +348,25 @@ enum PropertyType {
};
+inline bool IsTransitionType(PropertyType type) {
+ switch (type) {
+ case MAP_TRANSITION:
+ case CONSTANT_TRANSITION:
+ case ELEMENTS_TRANSITION:
+ return true;
+ case NORMAL:
+ case FIELD:
+ case CONSTANT_FUNCTION:
+ case CALLBACKS:
+ case HANDLER:
+ case INTERCEPTOR:
+ case NULL_DESCRIPTOR:
+ return false;
+ }
+ UNREACHABLE(); // keep the compiler happy
+ return false;
+}
+
// Whether to remove map transitions and constant transitions from a
// DescriptorArray.
enum TransitionFlag {
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev