Please review JDK-8168373 "don't emit conversions for symbols outside their 
lexical scope" at <http://cr.openjdk.java.net/~attila/8168373/webrev.jdk9> for 
<https://bugs.openjdk.java.net/browse/JDK-8168373>

Few remarks on the implementation:
- I changed localVariableTypes from using IdentityHashMap to HashMap. The 
reason for this is that Symbol uses identity semantics for equals/hashCode 
anyway, and IdentityHashMap.keySet() has a worse implementation of removeAll, 
which is now used. HashMap has O(min(m, n)) where “m” and “n” are sizes of the 
two collections involved in removeAll, while IdentityHashMap.keySet() is always 
O(n) to the size of the map.

- in the “-462,13” patch block: we now clone localVariableTypes at most once, 
instead of once per symbol that needs to be set in it (that was happening in 
the setType() call). This is an optimization fix.

- in the “-1044,19” patch block there’s no longer a need to manually remove 
exceptionSymbol from the localVariableTypes when following the CF edge from a 
catch body to the try statement's end label. It becomes part of the general 
case handling lexically scoped symbols; leaveBlock on catchBody will take care 
of it.

- the PrintVisitor change now correctly prints “const” and “let” nodes. I was 
using it for debugging and it was confusing that it printed “var” for “let”.

Thanks,
  Attila.

Reply via email to