Introduce HObjectAccess, which is used by LoadNamedField and StoreNamedField to denote what parts

src/hydrogen.h

Index: src/hydrogen.h
diff --git a/src/hydrogen.h b/src/hydrogen.h
index bc76cdd3de0aaee8cc1f76cfc54e15760bbde40b..64f17baa6b79bb215b81bfece96cc8176f07985c 100644
--- a/src/hydrogen.h
+++ b/src/hydrogen.h
@@ -1017,8 +1017,16 @@ class HGraphBuilder {
       KeyedAccessStoreMode store_mode,
       Representation checked_index_representation = Representation::None());
 
-  HInstruction* BuildStoreMap(HValue* object, HValue* map);
-  HInstruction* BuildStoreMap(HValue* object, Handle<Map> map);
+  HStoreNamedField* AddStore(HValue *object, ObjectAccess access, HValue *val);
+
+  ObjectAccess AccessArray(int offset);

[danno, 2013/04/26 09:39:38]

I think these methods are better suited as static methods on the ObjectAccess
class in order to encapsulate the logic mapping fields to their flags/etc. in
one class (even though it means passing the builder--or at least the isolate--as
a parameter):

ObjectAccess::ForArrayLength(this); 

[titzer, 2013/04/30 15:56:47]

I'm OK with putting the guts of those methods in ObjectAccess if they have to do
anything tricky, but as for hydrogen.cc, having these methods saves a lot of
characters, reducing the need for other intermediate build methods (e.g.
BuildStoreMap is now gone)

[danno, 2013/05/02 14:16:47]

Tricky/non-trick is an arbitrary judgment call, I don't think encapsulation
is... and it's binary. I feel pretty strongly that if we're going through all
this trouble to create a class to hide the details of mapping ObjectAccess to
portions/flags/whatever, all of the logic for that should be in on place... the
ObjectAccess class and not partially sprinkled through some methods on
HGraphBuilder. I don't think there should be any reason to expose the "portion"
of the ObjectAccess in the public API.

Also, "saves a lot of characters" doesn't seem like a very good argument for
choosing readable names. For example, I think "AccessInobject" doesn't tell me
much. AccessInobject what? Properties? If so, why not put that in the name?
Access is a verb, so does the method actually access something? I don't mean to
be pedantic, but for others reading the code for the first time, a few extra
characters can drastically improve readability if they are the right ones.


On 2013/04/30 15:56:47, titzer wrote:

[titzer, 2013/05/03 09:18:41]

I've moved these methods into static constructors in HObjectAccess. What I meant
by saving characters was that writing AccessX() within the GraphBuilder code is
shorter than writing HObjectAccess::X(zone()), because GraphBuilder already
closes over zone, and the code is already in the GraphBuilder scope. I did not
mean to argue X versus a more readable AccessX. But I'm OK with putting it all
in one place at the expense of needing HObjectAccess:: as a prefix.

+  ObjectAccess AccessArrayLength();
+  ObjectAccess AccessElements();
+  ObjectAccess AccessFixedArrayLength();
+  ObjectAccess AccessMap();
+  ObjectAccess AccessField(Handle<Map> map, Handle<String> name,

[danno, 2013/04/26 09:39:38]

Shouldn't need the name here, the LookupResult has it.

[titzer, 2013/04/30 15:56:47]

I hunted around inside the LookupResult and couldn't find it. Does it require
looking up in the map with the index result from the LookupResult?

+     LookupResult* lookup);
+  ObjectAccess AccessInobject(Handle<String> name, int offset);

[danno, 2013/04/26 09:39:38]

Should take a map here. Look up the name in the descriptor array if you need it.

[titzer, 2013/04/30 15:56:47]

We might need another version that takes a map when we move the ObjectAccess to
use maps.

 
   class IfBuilder {
    public: