Very nice that we can keep using switch statements!
+1
Hannes
Am 2015-10-21 um 12:48 schrieb Attila Szegedi:
Here’s an updated webrev, please review:
<http://cr.openjdk.java.net/~attila/8139931/webrev.jdk9.01>
I removed CompositeOperation.contains and containsAny operations and in Nashorn
replaced their use with a new
NashornCallSiteDescriptor.getFirstStandardOperation method; this allowed us to
go back to using switch statement everywhere in linker logic where we were able
to use it earlier, resulting in smaller diff and overall nicer code.
Also, following a discussion with Hannes, we now enforce in CompositeOperation
constructor that no components can themselves be either a CompositeOperation or
a NamedOperation, and we enforce in NamedOperation that its base operation
can’t itself be a NamedOperation.
Thanks,
Attila.
On Oct 20, 2015, at 12:29 PM, Sundararajan Athijegannathan
<sundararajan.athijegannat...@oracle.com> wrote:
+1 with changes.
On final modifier: nothing specific comes to mind. We can leave it non-final...
-Sundar
On 10/20/2015 3:09 PM, Attila Szegedi wrote:
On Oct 20, 2015, at 10:52 AM, Sundararajan Athijegannathan
<sundararajan.athijegannat...@oracle.com> wrote:
* StandardOperation.java is missing copyright.
* NamedOperation.java is missing copyright.
* CompositeOperation.java is missing copyright.
Indeed; added copyrights.
* Can CompositeOperation be final?
Well, it’s hard to see why would someone want to subclass it, but I also don’t
really see why someone shouldn’t be able to do it, to maybe add some
language-specific information and still have it be recognized externally as
CompositeOperation. (Same reasoning goes for NamedOperation, I guess). Do you
have an rationale for making it final?
* Unrelated ArrayData change? Unused method?
Yes. I was following a change in the parameter type from String to Operation
and stumbled upon it.
* NashornCallSiteDescriptor may have explanation as to why 18 bits are sufficient for
"program point" [now that flag bits are used for encoding operation enums as
well]
I agree, I added an explanation.
That's all I could find...
+1
-Sundar
On 10/20/2015 1:41 PM, Attila Szegedi wrote:
Please review JDK-8139931 "Introduce Operation objects in Dynalink instead of string
encoding" at <http://cr.openjdk.java.net/~attila/8139931/webrev.jdk9> for
<https://bugs.openjdk.java.net/browse/JDK-8139931>
This is admittedly a big one. It is also the last in the pipeline of the
internal Dynalink cleanups, so we’ve reached the end of that!
Thanks,
Attila.
