+1
On 11/10/2016 5:50 PM, Attila Szegedi wrote: > 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. >>>
