Variable scoping seems overly restrictive

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?

Top Replies

peterdedecker over 3 years ago +1

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…

All Replies

peterdedecker over 3 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

jim_m_58 over 3 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.

markw65 over 3 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...

waterkip over 3 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.

markw65 over 3 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.

Alan.raceQs over 3 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.

waterkip over 3 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
      }
}

Alan.raceQs over 3 years ago in reply to waterkip

Please test your code.

SDK 4.0.9 generates:

waterkip over 3 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.

waterkip over 3 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".