Bug in HoistTrans with loop scoped variables

[sergisiso]

There is a subtle bug in the HoistTrans that brings a statement outside of a loop when possible. The bug is when the statement accesses a symbol declared in the loop body symbol scope. This cannot be expressed in Fortran so we almost never see it but it is possible to do it in PSyIR.

The fix is simple, also hoist the symbol (and rename it if the name already exist in the outer scope). But then the symbol_table remove method does not work to remove the loop body original entry because it is supposed to only allow deletions if there are no remaining references to a symbol in order to avoid undeclared variables. But in this case we want undeclared variables because we know we will put the symbol back in the scope hierarchy.

I am wondering if it would be better to have 2 removes:

• safe_remove: Remove only if no undeclared references to the symbol are left.
• unsafe_remove: Remove and the programmer has to guarantee the resulting correctness.

@arporter @rupertford What do you think of these methods?

There is another problem that I haven't seen but could happen, that is if the symbol had a tag in the loop body symbol table. We could bring the tag upwards as well but we cannot resolve tag-name-clashes since the purpose of the tag is to identify the concept with a unique/unchanging name regardless of what the symbol name ends up doing. We might need to fail the Hoist at this point.

[arporter]

As discussed, I would prefer an ignore_references=True argument (or similar) to the remove method rather than an additional method.