jscert / jsexplain

Apache License 2.0
26 stars 4 forks source link

Psuedocode generation puts tuple bindings backwards #24

Closed IgnoredAmbience closed 6 years ago

IgnoredAmbience commented 6 years ago

On the proxies branch, tuple bindings are reversed from the actual order. This was 'fixed' in #16, but seemingly not for all outputs. The function that builds this data structure is a fold/map combo which was rather awkward to work with. Definitely needs review with a fresh pair of eyes.

For example, see the init_object function in JsInterpreter, although other examples likely exist too.

brabalan commented 6 years ago

What is the problem in init_object? The only tuple binding I see is this:

    let (pn, _term_) = p in

translated to

      var _term_ = p[1];var pn = p[0];

Although the order the binding is done is in reverse, the code seems to do the right thing.

IgnoredAmbience commented 6 years ago

The .psuedo js outputs the binders as (term, pn) =...

On 19 July 2018 09:05:08 BST, Alan Schmitt notifications@github.com wrote:

What is the problem in init_object? The only tuple binding I see is this:

   let (pn, _term_) = p in

translated to

     var _term_ = p[1];var pn = p[0];

Although the order the binding is done is in reverse, the code seems to do the right thing.

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/jscert/jsexplain/issues/24#issuecomment-406191303

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

brabalan commented 6 years ago

The problem is that tuple_component_bind (https://github.com/jscert/jsexplain/blob/es2016-proxies/generator/js_of_ast.ml#L741) uses a List.fold_left, hence it reverses the order of the binders (it's called from tuple_binders), so the ids at https://github.com/jscert/jsexplain/blob/es2016-proxies/generator/js_of_ast.ml#L874 are in reverse order.

This is not problematic for the generation of js because we use the index there.

Looking at the code, we have the same problem in monadic calls. This is confirmed in the var%_ret call in get_value.

I propose that we use a List.fold_right in tuple_component_bind. We don't care if it's not tail-rec (the list of binders is not that big).