• Replies 18 replies
  • Subscribers 22 subscribers
  • Views 2548 views
  • Users 0 members are here
Options
Related

Variable scoping seems overly restrictive

markw65
over 2 years ago

I've been trying to get a bunch of open source projects to compile, and a lot of the older ones have issues like:

function foo(bar) {
    do_something(bar);
    var bar = 0;
    ...
}

which results in "ERROR: Redefinition of variable 'bar'.

At first I thought the projects were just broken... but this is such a common pattern, across lots of different projects, that I'm assuming the behavior here has changed, and this used to compile successfully. Is there a hidden flag that would let it compile again?

I was also surprised to get errors for code like:

function foo(a) {
    var x = 0;
    if (a) {
        for (var x = 0; x < 10; x++) {
            // ...
        }
    }
}

While such code is asking for trouble, I'd expect more of a warning than an error.

Finally I found one interesting case thats sort of a combo:

function foo(a, i) {
    for (var i = 0; i < a.size(); i++) {
        // do something
    }
    // a is an array
    a.add(i);
}

Since I'd been mechanically fixing a few of these, my initial fix was to drop the "var" on the i in the for loop, but then we end up adding a number to the end of the array, when i was in fact a string. Doing this broke the app in such an obvious way that I have to think that at one time, this actually "worked". The i in the for loop was scoped to the for loop, and a.add(i) added the original value of i to the array.

Can anyone comment on whether the behavior really has changed? And if there are flags to get the old behavior back?

  • over 2 years ago

    Yes this changed a few months(/year) ago in one of the sdks. I also had similar code in my projects i needed to fix. 

    There's no flag that I'm aware of to still allow redefinition of a variable with more limited scope 

  • over 2 years ago in reply to peterdedecker

    Yes, it's been a while.  Maybe about the time Monkey Types or the VS Code debugger was introduced.  I had a few cases where I had to do a minor change to some code.

  • over 2 years ago

    Thanks. Since my goal in looking at these projects was to get more test cases for my monkeyc optimizer, and there were too many of these issues in the various open source projects I found to fix manually, I ended up just adding a fix in the optimizer; when it finds such conflicts, it renames the inner scoped one. It does mean that many of the projects won't build un-optimized, but will build optimized, but thats ok as far as my testing goes...

  • over 2 years ago

    Seems normal scoping to me:

    function foo(bar) {
  // bar is in scope
  var bar = 'bar'; // you are now redefining bar inside the scope
  var foo = 'foo';
  for (var foo = 0; foo < 5; foo++) { // the for-loop is a new scope, thus you can create
                                      // a new foo var
     var bar = 'barry'; // untested but.. should be legit, new scope because in for-loop, 
                        // doesn't mess with the higher scoped bar
  }
}

    I would argue that reusing variable names is an incorrect pattern in most cases.

  • over 2 years ago in reply to waterkip

    The problem is that, as you said, you didn’t actually try it. And in fact all of your examples are illegal, even where the variables are clearly in different scopes (the parameter vs local behavior is indeed dubious, but in older versions of the compiler it worked). 

    And yes, it’s generally not a good idea to do it (I said so in my post). But there is lots of code out there that does (I imagine that most cases are simply because the author didn’t notice, and the compiler didn’t complain). A warning (or optional error) would have been a much better approach to this. 

  • over 2 years ago in reply to waterkip

    It's really a good idea to test code before presenting it as an example.

    Your code fails in compile with

    Variable bar already declared at this scope level.

  • over 2 years ago in reply to markw65

    I see your point, I miss read your post in that case.

    function foo(bar) {
    var bar = "bar"; // bar is in scope, and you are now redefining bar inside
                     // the scope. The error is legit.
    var foo = "foo";
    for (var foo = 0; foo < 5; foo++) { // the for-loop is a new scope, thus
                                        // you should be able to create a new
                                        // foo var.
        var bar = "barry"; // currently an error but should be legit, new scope
                           // because in for-loop doesn't mess with the higher
                           // scoped bar
      }
}

  • over 2 years ago in reply to waterkip

    Please test your code.

    SDK 4.0.9 generates:

  • over 2 years ago in reply to Alan.raceQs

    Ever heard of pseudo code? It mimics the real world to explain a concept yet isn't 100% correct. My example fails on the third line because it redefines/redeclares bar.

  • over 2 years ago in reply to Alan.raceQs

    Yes, I know, I'm pseudo coding here. Look at the words "The error is legit", "SHOULD be able" and "Currently an error but SHOULD be legit".