function hoisting

[gfwilliams]

When parsing function decls, parse and remove other function decls inside them.

This should have the affect of hoisting, and will speed up execution too!

[gfwilliams]

Could be more difficult when converting functions back into strings though

[KeithJRome]

Converting functions back into source strings (using dump() for example) already doesn't work for nontrivial code. A simple example:

var foo_module = (function(){
  var foo = "hello, world from a secret place!";
  var bar = function() { console.log(foo); };
  return { 'bar': bar };
})();

This instantiates an object named foo_module using the revealing module pattern - which really just uses closures to simulate "private" members. In this example, "foo" is a private field of "foo_module", and "bar" is a public member that proves "foo" is still there, even though you cannot directly reach "foo". This is a very common pattern in real-world javascript code, almost necessary for projects of appreciable size.

>foo_module.bar()
hello, world from a secret place!
=undefined
>dump()
var foo_module = {
  "bar":function () { console.log(foo); }
};
=undefined
>foo_module.foo
=undefined
> 

Note that the last statement there actually adds a public undefined "foo" member to "foo_module", which is surely a bug. It looks like during the process of looking up a member, espruino is actually creating the member if it doesn't already exist:

>dump()
var foo_module = {
  "bar":function () { console.log(foo); },
  "foo":undefined};
=undefined
>

[rwaldron]

Not sure I follow the explanation given, re: parsing and removing? Can you clarify? Functions don't hoist beyond the function scope (or global scope) that they are declared in, maybe I'm not following correctly?

[gfwilliams]

@KeithJRome That's already covered on these two (6 month old) bugs: #35 and #63. Both of them are really easy to find, so please search next time. If you want to fix them, pull requests are always appreciated ;)

@rwaldron yeah - it's more a note for me :) The idea is to turn this (approximation of Espruino's symbol table) :

{
  a : {
   code : "function b() { print('hello'); } print('bar'); b();"
  }
 }

into:

{
  a : {
   b : { code : "print('hello');" },
   code : "print('bar'); b();"
  }
 }

Obviously then you have an issue when converting that function back into a string, for example for edit() or dump() - but for now, the declared functions could just be dumped right at the top.

[rwaldron]

@gfwilliams ah, got it.

[KeithJRome]

I had actually seen those before; I wasn't attempting to report new bugs. I was simply pointing out that your concern ("Could be more difficult when converting functions back into strings though") didn't really matter until/if the other was dealt with first. The second bug mentioned (accessing a nonexistent member) was brought up simply because anyone who tries that code snippet I posted would likely notice it and I didn't want that to overshadow what my actual point was.

[KeithJRome]

Your point is taken though. I'll try to make some time to contribute an actual pull request or two. I've just not had the bandwidth for it yet due to an extremely heavy workload at the moment.

[bartmichu]

I was looking for a reason why this example code fails and it looks like it's the right place.

function test(){
  function f() {
    return "not very good";
  }
  return f();
  function f() {
    return "yay";
  }
}

It fails with:

>test()
at line 6 col 3
  function f() {
   ^
in function "test" called from line 1 col 6
>

[gfwilliams]

Hmm. This looks like a slightly different bug though - while it would get the wrong result, it shouldn't error. I've just added a new one for it.

[gfwilliams]

When implementing this: There's a kind of variable hoisting too - it just hoists undefined:

var foo = 2;
function a() {
  return foo;
  var foo = 4;
}
console.log(a());
// undefined

this could probably be implemented in almost the same way:

• keeping the original variable decl
• define a variable inside the function
• when prepping an execution context (parsing arguments into local variables), make sure we create a new undefined variable

[rm-rf-etc]

Wow, was really surprised to see function hoisting isn't working. This will break any JS modules that use hoisting, which I assume is a lot. Wanted to provide a code sample and the error I see.

var UI = require('morainterface');
var board = new UI();
board.addButton(B3, makeCallback(B3));
board.addButton(B4, makeCallback(B4));
board.addButton(B5, makeCallback(B5));

function makeCallback(name){

    name = name.toString();
    return function(e) {
        console.log(name, e.state);
    };
}

Uncaught ReferenceError: "makeCallback" is not defined
 at line 1 col 21
board.addButton(B3, makeCallback(B3));
                    ^
Uncaught ReferenceError: "makeCallback" is not defined
 at line 1 col 21
board.addButton(B4, makeCallback(B4));
                    ^
Uncaught ReferenceError: "makeCallback" is not defined
 at line 1 col 21
board.addButton(B5, makeCallback(B5));
                    ^
=undefined

[gfwilliams]

It's surprisingly few modules, because you only hit it if you execute the code at the root scope of the module (so it executes as soon as the module is loaded), which seems like bad practice anyway. If you put your initialisation in a function - which, as you're making a UI, you might want to do after the screen is initialised - you wouldn't have the problem.

From the errors you're showing, you're not uploading a module, but normal JS code from the Web IDE?

In that case, it's impossible to fix. Each command is executed in sequence, as soon as it's loaded into Espruino - it's done so that you can load a text file that is bigger than might otherwise fit in Espruino's memory if it were loaded as one chunk... But that means that when Espruino executes board.addButton(B3, makeCallback(B3)); it has never even received the characters for makeCallback and has no idea what it is.

[trusktr]

Interesting. I stumbled on this too. But I can appreciate why it works the way it does while scanning the code in one go.