2nd review, anyone? Or can I go ahead with one? Attila.
> On 08 Nov 2016, at 18:07, Hannes Wallnöfer <hannes.wallnoe...@oracle.com> > wrote: > > +1 > > Thank for fixing this, Attila! > > Hannes > >> Am 08.11.2016 um 14:01 schrieb Attila Szegedi <szege...@gmail.com>: >> >> 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. >> >
