Order of declarations affects shadowing error reporting

[apblack]

This program

method foo(secret) { secret }
method secret { ... }

should report a shadowing error when method foo uses secret as the name of its parameter. If the order of the declarations is reversed, the error is correctly reported:

method secret { ... }
method foo(secret) { secret }

Syntax error: 'secret' cannot be redeclared because it is already declared in an enclosing module scope on line 1. Use a different name.

This bug occurs because the declarations are processed in the order written, and shadowing errors are detected when the declaration of a temp or a parameter is processed. What ought to happen is that the lhs of all of the declarations are processed before the rhs of any of them. Not clear how to do this in a visitor!

Perhaps the simplest thing is to defer shadowing error reporting until after the symbol table has been built.

[kjx]

What ought to happen is that the lhs of all of the declarations are processed before the rhs of any of them. Not clear how to do this in a visitor!

which is why I didn't do it in a visitor. Evaluating anything that can hold declarations (methods, blocks, objects, etc) must be done in two steps: first build the declarations into the context, second evaluate their values. That's two mutually recursive methods, so presumably two mutually recursive visitors.

[apblack]

That's two mutually recursive methods

No, just two passes at compile time; first build the symbol tables, and then check for shadowing. I don't know why I didn't see this before; it's another example of not understanding how to do something until after you have done it.

[apblack]

@Kim entered a duplicate of this as #260. That issue contains a larger example.