[Webkit-unassigned] [Bug 28944] Squirrelfish bytecode documentation is out of date.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=28944





--- Comment #1 from Darin Adler <darin at apple.com>  2009-09-03 10:26:38 PDT ---
(From update of attachment 38983)
> +<h2><code>eq_null</code></h2>
> +<p><b>Format: </b><code>
> +eq_null dst(r) src(r)
> +</code></p>
> +<p>
> +
> +            Checks whether register src is null, as with the ECMAScript '!='
> +            operator, and puts the result as a boolean in register dst.

'==', not '!='

> +            Determines whether the type string for src according to
> +            the ECMAScript rules is "undefined", and puts the result
> +            in register dst.

Saying this in terms of "type string" for these opcodes seems like an oblique
way to say that this checks if the type is undefined. Is there some reason you
need to do that to be precise or accurate?

> +<h2><code>resolve_global</code></h2>
> +<p><b>Format: </b><code>
> +resolve_skip dst(r) globalObject(c) property(id) structure(sID) offset(n)

resolve_global, not resolve_skip

> +</code></p>
> +<p>
> +         
> +            Performs a dynamic property lookup for the given property, on the provided
> +            global object.  If structure matches the Structure of the global then perform
> +            a fast lookup using the case offset, otherwise fall back to a full resolve and
> +            cache the new structure and offset

Missing a period at the end. We use single spaces after periods in this
project.

> +</p>
> +<h2><code>get_global_var</code></h2>
> +<p><b>Format: </b><code>
> +get_global_var dst(r) globalObject(c) index(n)
> +</code></p>
> +<p>
> +
> +            Gets the global var at global slot index and places it in register dst.

We should say "global variable", not "global var".

> +</p>
> +<h2><code>put_global_var</code></h2>
> +<p><b>Format: </b><code>
> +put_global_var globalObject(c) index(n) value(r)
> +</code></p>
> +<p>
> +         
> +            Puts value into global slot index.

In get_global_var you mention "global variable" but here you just say "put
into". I think the wording should be consistent. You could just say "Gets the
value from global slot index" or you could say "global variable at global slot
index" here.

> +<h2><code>get_by_id_self</code></h2>
>  <p><b>Format: </b><code>
> +op_get_by_id_self dst(r) base(r) property(id) structure(sID) offset(n) nop(n) nop(n)

get_by_id_self, not op_get_by_id_self

No discussion of the structure and offset arguments. They should be documented.


> +            Cached property access: Attempts to get a cached property from the
> +            value base. If the cache misses, op_get_by_id_self reverts to
> +            op_get_by_id.

The uses of the op_ prefix here seem unnecessary. The terminology "reverts to"
is a little confusing. It's strange that in other opcodes we explain each
argument but here we don't.

Sorry, I don't have time to review all the rest of this.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.