Re: Fwd: Re: [PATCH] javax,script.ScriptEngineFactory Typos
Hi Ahmed,
Did you build the forest with that change or just "found" by code
reading? Because {@code requires } to end it. So, those were not 'extra'
'}' chars in doc comments.
Also, you can check javadoc output is correct. i.e., no extra "}".
http://docs.oracle.com/javase/8/docs/api/javax/script/ScriptEngineFactory.html#getProgram-java.lang.String...-
The other diffs being simple white space removals, I/we'll take care as
part of another fix.
Thanks,
-Sundar
On Tuesday 14 July 2015 09:30 AM, A. Sundararajan wrote:
Forwarding this contribution from Ahmed to core-libs-dev alias as the
change is going to be in "jdk/java.scripting/javax.script" code.
PS. I'll send out webrev after build, test.
Thanks Ahmed,
-Sundar
Forwarded Message
Subject: Re: [PATCH] javax,script.ScriptEngineFactory Typos
Date: Fri, 10 Jul 2015 07:46:40 +0200
From: Ahmed Ashour
To: nashorn-...@openjdk.java.net
Dear all,
Please find below a proposed patch based on jdk9/dev.
Thanks,
Ahmed
diff -r b526c2584b4b
src/java.scripting/share/classes/javax/script/ScriptEngineFactory.java
---
a/src/java.scripting/share/classes/javax/script/ScriptEngineFactory.java
Wed Jul 08 21:54:32 2015 -0400
+++
b/src/java.scripting/share/classes/javax/script/ScriptEngineFactory.java
Thu Jul 09 08:10:26 2015 +0200
@@ -160,7 +160,6 @@
* }
* ret += ")";
* return ret;
- * }
* }
*
* @param obj The name representing the object whose method is to
be invoked. The
@@ -190,8 +189,6 @@
*
* @param toDisplay The String to be displayed by the returned
statement.
* @return The string used to display the String in the syntax of
the scripting language.
- *
- *
*/
public String getOutputStatement(String toDisplay);
@@ -208,14 +205,12 @@
* retval += statements[i] + ";\n";
* }
* return retval += "?>";
- * }
* }
*
* @param statements The statements to be executed. May be
return values of
* calls to the getMethodCallSyntax and
getOutputStatement methods.
* @return The Program
*/
-
public String getProgram(String... statements);
/**
@@ -225,5 +220,5 @@
*
* @return A new ScriptEngine instance.
*/
-public ScriptEngine getScriptEngine();
+public ScriptEngine getScriptEngine();
}
Fwd: Re: [PATCH] javax,script.ScriptEngineFactory Typos
Forwarding this contribution from Ahmed to core-libs-dev alias as the change is going to be in "jdk/java.scripting/javax.script" code. PS. I'll send out webrev after build, test. Thanks Ahmed, -Sundar Forwarded Message Subject:Re: [PATCH] javax,script.ScriptEngineFactory Typos Date: Fri, 10 Jul 2015 07:46:40 +0200 From: Ahmed Ashour To: nashorn-...@openjdk.java.net Dear all, Please find below a proposed patch based on jdk9/dev. Thanks, Ahmed diff -r b526c2584b4b src/java.scripting/share/classes/javax/script/ScriptEngineFactory.java --- a/src/java.scripting/share/classes/javax/script/ScriptEngineFactory.java Wed Jul 08 21:54:32 2015 -0400 +++ b/src/java.scripting/share/classes/javax/script/ScriptEngineFactory.java Thu Jul 09 08:10:26 2015 +0200 @@ -160,7 +160,6 @@ * } * ret += ")"; * return ret; - * } * } * * @param obj The name representing the object whose method is to be invoked. The @@ -190,8 +189,6 @@ * * @param toDisplay The String to be displayed by the returned statement. * @return The string used to display the String in the syntax of the scripting language. - * - * */ public String getOutputStatement(String toDisplay); @@ -208,14 +205,12 @@ * retval += statements[i] + ";\n"; * } * return retval += "?>"; - * } * } * * @param statements The statements to be executed. May be return values of * calls to the getMethodCallSyntax and getOutputStatement methods. * @return The Program */ - public String getProgram(String... statements); /** @@ -225,5 +220,5 @@ * * @return A new ScriptEngine instance. */ -public ScriptEngine getScriptEngine(); +public ScriptEngine getScriptEngine(); }
Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs
My understanding is that the new file won't have old copyright year (2011 in this case). -Sundar On Thursday 18 June 2015 09:20 PM, Jan Lahoda wrote: On 18.6.2015 16:40, A. Sundararajan wrote: * jdk/make/lib/Lib-jdk.jline.gmk has copyright year 2011, 2015 despite being a new file. Any specific reason? I copied one of the existing files in the directory to just adjusted it for jdk.jline. So I kept the copyright years there - what is the right thing to do here? * jdk.jline depends on java.desktop. Is that needed by the code by jline code? I am asking because Nashorn requires only "compact 1" profile so far and so can be used on compact1 embedded platforms. If desktop dependency is needed, I guess nashorn has to use it reflectively... JLine uses java.awt.event.ActionListener (which can be registered as callbacks - this could be somewhat tricky), java.awt.datatransfer (to implement paste), and java.awt.Toolkit (to get the Clipboard). I can take a look what could be done about that if needed. Thanks for the comments! Jan +1 from nashorn point of view. Thanks, -Sundar On Thursday 18 June 2015 07:55 PM, Jan Lahoda wrote: Hello, I am proposing to add JLine 2.12.1 into the jdk repository for use by the Java and Nashorn REPLs. Full patch is available here: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.00/full/ To aid the review, I've split this patch into to smaller patches: -a patch that only adds unmodified jline sources at appropriate places in the jdk repository: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.00/clean-jline/ -a patch that shows the additional changes I've done: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.00/additional/ This split is intended solely to simplify reviewing, my plan is to integrate this as a single patch. The main additional changes are: -plugging the new module, jdk.jline, into the JDK build. Currently, the JLine packages are exported only to jdk.scripting.nashorn (the plan is to also export them to the future jdk.jshell module). (The patch is not adding the dependency from jdk.scripting.nashorn to jdk.jline, though - I expect that to be added when needed.) -the sources are re-packaged from package "jline" to "jdk.internal.jline" -removing trailing whitespace, adding newlines at the end of the files, encoding characters that are not ASCII -avoiding the dependency on another library, jansi, by reimplementing two elements that were used from the other library. These are mainly the changes in WindowsTerminal and ConsoleReader.java. This also includes the WindowsTerminal.cpp native library. The native part is heavily inspired by: http://cr.openjdk.java.net/~sherman/rl/src/java.base/windows/native/libjava/Console_md.c.html As I am not experienced in native programming, comments to the native part would be particularly useful. -changes to resolve javac warnings in JLine. -tests for some of the added functionality. Any comments are welcome! Thanks, Jan
Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs
* jdk/make/lib/Lib-jdk.jline.gmk has copyright year 2011, 2015 despite being a new file. Any specific reason? * jdk.jline depends on java.desktop. Is that needed by the code by jline code? I am asking because Nashorn requires only "compact 1" profile so far and so can be used on compact1 embedded platforms. If desktop dependency is needed, I guess nashorn has to use it reflectively... +1 from nashorn point of view. Thanks, -Sundar On Thursday 18 June 2015 07:55 PM, Jan Lahoda wrote: Hello, I am proposing to add JLine 2.12.1 into the jdk repository for use by the Java and Nashorn REPLs. Full patch is available here: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.00/full/ To aid the review, I've split this patch into to smaller patches: -a patch that only adds unmodified jline sources at appropriate places in the jdk repository: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.00/clean-jline/ -a patch that shows the additional changes I've done: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.00/additional/ This split is intended solely to simplify reviewing, my plan is to integrate this as a single patch. The main additional changes are: -plugging the new module, jdk.jline, into the JDK build. Currently, the JLine packages are exported only to jdk.scripting.nashorn (the plan is to also export them to the future jdk.jshell module). (The patch is not adding the dependency from jdk.scripting.nashorn to jdk.jline, though - I expect that to be added when needed.) -the sources are re-packaged from package "jline" to "jdk.internal.jline" -removing trailing whitespace, adding newlines at the end of the files, encoding characters that are not ASCII -avoiding the dependency on another library, jansi, by reimplementing two elements that were used from the other library. These are mainly the changes in WindowsTerminal and ConsoleReader.java. This also includes the WindowsTerminal.cpp native library. The native part is heavily inspired by: http://cr.openjdk.java.net/~sherman/rl/src/java.base/windows/native/libjava/Console_md.c.html As I am not experienced in native programming, comments to the native part would be particularly useful. -changes to resolve javac warnings in JLine. -tests for some of the added functionality. Any comments are welcome! Thanks, Jan
Re: RFR: JDK-8085822 JEP 223: New Version-String Scheme (initial integration)
+1 on Nashorn changes. -Sundar On Monday 08 June 2015 06:07 PM, Magnus Ihse Bursie wrote: 8 jun 2015 kl. 11:34 skrev Alan Bateman : On 05/06/2015 15:07, Magnus Ihse Bursie wrote: This review request covers the main part of the work for JEP-223, the new version string format [1]. Basically, we'll call this release Java "9", instead of Java "1.9.0". This patch is a folding of all work that has been done so far in the branch JEP-223-branch in jdk9/sandbox. As you can see, it mostly covers build changes, with some code changes in hotspot, jdk, nashorn and langtools that either are corresponding changes in the product code due to the compiler define flags changing from the build, or follow-up changes to handle the new format. The JEP-223 work is not finished by this patch. In fact, there are known issues remaining even after this patch, typically by code that reads the "java.version" property and tries to parse it. However, this patch is not directly destined for jdk9/dev, but will go into the special verona/stage forest. As for all patches destined for verona/stage it will be code reviewed as if going to jdk9/dev. Once in verona/stage it will bide its time, and it will be complemented with follow-up patches to address remaining issues. When all such issues are resolved and JEP-223 is fully implemented, all changes will be pushed at once (without further code reviews) into jdk9/dev. This patch has been contributed by Magnus Ihse Bursie, Kumar Srinivasan and Alejandro Murillo. Bug: https://bugs.openjdk.java.net/browse/JDK-8085822 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8085822-JEP-223-initial-patch/webrev.01 I looked through the code changes, skipping most of the make files :-) Version.java.template - the comment in jvmSecurityVersion() still talks about 1.6 and newer. Can this be replaced to just say that it returns the security version? Will the update_version and special_update_version fields eventually be dropped from the jvm_version_info stricture? Related, there seems to be a typo in the comment in jdk_util.c where it has "specia_update_version". The webrev shows a change to this comment in jvm.h: "Third, this file contains various I/O and network operations needed by the standard Java I/O and network APIs." I think this comment can be removed because those JVM_* functions were removed some time ago. Otherwise looks okay to me. The API functions in Version.java and jvm.h are not finished. The specification in the JEP talks about a java.util.Version, that I presume will replace the sun.misc.Version, and that will fully implement an API to access the version string and all it's parts, according to the JEP definition. Also, the native interface will have to be changed to accommodate a version number with an arbitrarily number of dot separated parts. These changes will be done later on in the verona/stage forest. Are you ok with addressing these concerns at such a later time? /Magnus -Alan.
RFR 8068978: All versions of javax.script.ScriptEngine.eval(...) method may clarify ScriptException throwing
Please review http://cr.openjdk.java.net/~sundar/8068978/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8068978 Thanks, -Sundar
RFR 8072002: The spec on javax.script.Compilable contains a typo and confusing inconsistency
Please review http://cr.openjdk.java.net/~sundar/8072002/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8072002 Thanks, -Sundar
Re: RFR 8072853: SimpleScriptContext used by NashornScriptEngine doesn't completely complies to the spec regarding exception throwing
Thanks for the review. Updated test as per your suggestion. Uploaded
fresh review @ http://cr.openjdk.java.net/~sundar/8072853/webrev.01/
Thanks
-Sundar
Paul Sandoz wrote:
On May 18, 2015, at 12:44 PM, A. Sundararajan
wrote:
Please review http://cr.openjdk.java.net/~sundar/8072853/webrev.00/ for
https://bugs.openjdk.java.net/browse/JDK-8072853
Changes to SimpleScriptContext look good.
Test-wise you could reduce the duplication with a method accepting
Consumer.
e.g.
@Test
public void getAttributeEmptyName() {
test(sc -> sc.getAttribute("", ScriptContext.GLOBAL_SCOPE));
}
void test(Consumer c) {
for (ScriptEngineFactory fac : getFactories()) {
ScriptContext sc = fac.getScriptEngine().getContext();
String name = fac.getEngineName();
try {
c.accept(sc);
throw new RuntimeException("no exception for " + name);
} catch (IllegalArgumentException iae) {
System.out.println("got " + iae + " as expected for " + name);
}
}
}
Paul.
RFR 8072853: SimpleScriptContext used by NashornScriptEngine doesn't completely complies to the spec regarding exception throwing
Please review http://cr.openjdk.java.net/~sundar/8072853/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8072853 Thanks, -Sundar
RFR 8068587: ScriptEngineFactory.getParameter() should specify NPE for a null key
Please review http://cr.openjdk.java.net/~sundar/8068587/ for https://bugs.openjdk.java.net/browse/JDK-8068587 Thanks, -Sundar
Re: RFR 8068462: javax.script.ScriptEngineFactory.getParameter spec is not completely consistent with the rest of the API
Thanks. Broke that sentence into two for clarity. -Sundar On Tuesday 06 January 2015 04:46 PM, Alan Bateman wrote: On 06/01/2015 07:57, A. Sundararajan wrote: Please review http://cr.openjdk.java.net/~sundar/8068462/ for https://bugs.openjdk.java.net/browse/JDK-8068462 I think this is okay and doesn't require an update to JSR 223. One suggestion (feel free to ignore) to to split the sentence into two so that the Strings returned by getNames is in a second statement. -Alan.
RFR 8068462: javax.script.ScriptEngineFactory.getParameter spec is not completely consistent with the rest of the API
Please review http://cr.openjdk.java.net/~sundar/8068462/ for https://bugs.openjdk.java.net/browse/JDK-8068462 Thanks, -Sundar
RFR 8068279: (typo in the spec) javax.script.ScriptEngineFactory.getLanguageName
Please review a typo in javadoc comment.. http://cr.openjdk.java.net/~sundar/8068279/ for https://bugs.openjdk.java.net/browse/JDK-8068279 Thanks, -Sundar
Re: RFR: 8061830: [asm] refresh internal ASM version v5.0.3
+1 -Sundar On Wednesday 22 October 2014 08:39 PM, Kumar Srinivasan wrote: Hello, Please review fix for JDK-8061830, this is merely a refresh of the existing source base from upstream ObjectWeb/ASM, the webrev is here: http://cr.openjdk.java.net/~ksrini/8061830/webrev.00/ All the validations performed are documented in the JBS issue. Thanks Kumar
RFR 8044647: sun/tools/jrunscript/jrunscriptTest.sh start failing: Output of jrunscript -l nashorn differ from expected output
Hi, Please review http://cr.openjdk.java.net/~sundar/8044647/ Thanks, -Sundar
Re: Please review: 8044046: [asm] refresh internal ASM version to v5.0.3
Looks good to me. -Sundar On Friday 30 May 2014 08:23 PM, Paul Sandoz wrote: On May 28, 2014, at 1:20 AM, Kumar Srinivasan wrote: Hello, Please review the following webrev which updates internal ASM to v5.0.3, the individual bug fixes are listed in the JBS issue for reference, https://bugs.openjdk.java.net/browse/JDK-8044046#comment-13501358 All core regression tests have been run, additionally nashorn regressions, test262parallel, octane, and all applicable JFR related tests have also been run. https://bugs.openjdk.java.net/browse/JDK-8044046 http://cr.openjdk.java.net/~ksrini/8044046/jdk9/webrev.00/ Code changes look OK ro me. Paul.
Re: 8034780: Remove used imports
+1 -Sundar On Wednesday 12 February 2014 06:49 PM, Alan Bateman wrote: I need a reviewer for a trivial change to remove a tiny number of unused imports. The motive is experimental compilation of the JDK as modules rather than one big compilation unit. I've no doubt that there is other code that has unused imports but it's not the goal of this exercise to identify and remove these. The proposal patch is below. -Alan diff --git a/src/share/classes/java/lang/invoke/MethodHandle.java b/src/share/classes/java/lang/invoke/MethodHandle.java --- a/src/share/classes/java/lang/invoke/MethodHandle.java +++ b/src/share/classes/java/lang/invoke/MethodHandle.java @@ -31,8 +31,6 @@ import sun.misc.Unsafe; import static java.lang.invoke.MethodHandleStatics.*; -import java.util.logging.Level; -import java.util.logging.Logger; /** * A method handle is a typed, directly executable reference to an underlying method, diff --git a/src/share/classes/java/lang/invoke/SimpleMethodHandle.java b/src/share/classes/java/lang/invoke/SimpleMethodHandle.java --- a/src/share/classes/java/lang/invoke/SimpleMethodHandle.java +++ b/src/share/classes/java/lang/invoke/SimpleMethodHandle.java @@ -27,8 +27,6 @@ import static java.lang.invoke.LambdaForm.*; import static java.lang.invoke.MethodHandleNatives.Constants.*; -import java.util.logging.Level; -import java.util.logging.Logger; /** * A method handle whose behavior is determined only by its LambdaForm. diff --git a/src/share/classes/sun/applet/AppletViewerPanel.java b/src/share/classes/sun/applet/AppletViewerPanel.java --- a/src/share/classes/sun/applet/AppletViewerPanel.java +++ b/src/share/classes/sun/applet/AppletViewerPanel.java @@ -31,7 +31,6 @@ import java.net.MalformedURLException; import java.awt.*; import java.applet.*; -import sun.tools.jar.*;
Re: Time to remove sun.misc.Service?
Looks good. -Sundar On Wednesday 12 February 2014 06:17 PM, Alan Bateman wrote: On 11/02/2014 11:44, Paul Sandoz wrote: : Scrub it! AFAICT it is not widely used (looking at the use of s.m.Service static methods on grep code there are only a handful of dependent artifacts). And the upgrade is quite easy. I didn't see any more comments on this and I agree there isn't much of a case for keeping it around. Here's the webrev to remove it: http://cr.openjdk.java.net/~alanb/8034776/webrev/ -Alan.
Re: [JDK8] RFR (XS): JSR292: IncompatibleClassChangeError in LambdaForm for CharSequence.toString() method handle type converter
The test sets compile threshold to be zero (-Djava.lang.invoke.MethodHandle.COMPILE_THRESHOLD=0 ). I think compilation occurs on the first invoke. Also, I ran the test on a jdk8 build without Vladimir's fix - I saw the exception being thrown. I ran it by passing the above option in the command line (outside jtreg). -Sundar On Thursday 16 January 2014 02:10 PM, Jochen Theodorou wrote: Hi, since I am indirectly the reporter of this bug I have one remark for the test. The error happens only for compiled lambda forms. The given test does imho not use a compiled lambda form. In other words, afaik the test would pass without the fix. As such it would be useless as regression test. Am 15.01.2014 16:31, schrieb Vladimir Ivanov: http://cr.openjdk.java.net/~vlivanov/8031502/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8031502 InvokeBytecodeGenerator can produce incorrect bytecode for a LambdaForm when invoking a method from Object declared in an interface. The problem is the following: (1) java.lang.CharSequence interface declares abstract method "String toString()"; (2) after 8014013 fix, VM resolves CharSequence::toString()/invokeInterface to CharSequence::toString()/invokeVirtual; (3) during LambdaForm compilation, CharSequence is considered statically invocable (see InvokeBytecodeGenerator::isStaticallyInvocable) and invokevirtual for CharSequence::toString() is issued, which is wrong (invokevirtual throws ICCE if it references an interface); The fix is straightforward: during LambdaForm compilation, switch back from invokevirtual to invokeinterface instruction when invoking a method on an interface. The fix is targeted for 8. Will be also integrated into 9. Testing: regression test, jdk/test/java/lang/invoke, vm.mlvm.testlist, nashorn, jruby. Thanks! Best regards, Vladimir Ivanov ___ mlvm-dev mailing list mlvm-...@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
Re: [JDK8] RFR (XS): JSR292: IncompatibleClassChangeError in LambdaForm for CharSequence.toString() method handle type converter
Looks good to me -Sundar On Wednesday 15 January 2014 09:01 PM, Vladimir Ivanov wrote: http://cr.openjdk.java.net/~vlivanov/8031502/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8031502 InvokeBytecodeGenerator can produce incorrect bytecode for a LambdaForm when invoking a method from Object declared in an interface. The problem is the following: (1) java.lang.CharSequence interface declares abstract method "String toString()"; (2) after 8014013 fix, VM resolves CharSequence::toString()/invokeInterface to CharSequence::toString()/invokeVirtual; (3) during LambdaForm compilation, CharSequence is considered statically invocable (see InvokeBytecodeGenerator::isStaticallyInvocable) and invokevirtual for CharSequence::toString() is issued, which is wrong (invokevirtual throws ICCE if it references an interface); The fix is straightforward: during LambdaForm compilation, switch back from invokevirtual to invokeinterface instruction when invoking a method on an interface. The fix is targeted for 8. Will be also integrated into 9. Testing: regression test, jdk/test/java/lang/invoke, vm.mlvm.testlist, nashorn, jruby. Thanks! Best regards, Vladimir Ivanov
Re: Replacement of sun.reflect.Reflection#getCallerClass
Agree. I was just pointing that there are 'sensitive' packages and access to sensitive package classes - both normal linking reference and reflective reference by Class.forName - is security access checked. (i.e., there are Class objects that are security access protected as well - not just ClassLoader instances). -Sundar On Tuesday 03 September 2013 11:03 PM, Jochen Theodorou wrote: Am 03.09.2013 16:12, schrieb A. Sundararajan: [...] If Groovy or any third-party framework gets away with that -- that is because you need to use modified security policy that gives those necessary permissions to groovy.jar or whatever third-party jar in question. just think of us needing to build a runtime structure "copying" what is in a normal class (plus some changes) available in terms of fields and methods. If you don't generate that information (and you cannot for unknown classes), then how can you get that without using reflection and things like getDeclaredMethods. (not to mention several properties and many other things). In other words: it is imho impossible to run even a single Groovy program without giving it some permissions. bye Jochen
Re: Replacement of sun.reflect.Reflection#getCallerClass
I don't think it is possible to get every Class object in the system.
Either you should have a reference to an object (in which case you can
call Object.getClass method) or the classloader that loaded your class
should be able to resolve the referred class.
For example,
Class.forName("sun.misc.Unsafe")
will fail with AccessControlException (when running under security
manager with default policy).
Even
Class c = sun.misc.Unsafe.class;
will fail.
If Groovy or any third-party framework gets away with that -- that is
because you need to use modified security policy that gives those
necessary permissions to groovy.jar or whatever third-party jar in question.
-Sundar
On Tuesday 03 September 2013 06:22 PM, Nick Williams wrote:
On Sep 2, 2013, at 10:04 PM, Mandy Chung wrote:
Hi Nick,
Thanks for the patch.
JEP 176 [1] describes the caller-sensitive method and the need for a mechanical
checking of caller-sensitive methods. Also Peter Levart in [2] explained the
change in MethodHandles.Lookup related to @CS. I assume you understand the
rationale behind and the potential security issues. In general defining
caller-sensitive API is discouraged.
Do, I don't understand the rationale. Alan said the security issues couldn't be
discussed openly. I can get a Class object MANY different ways without a
security check. I don't see or understand any vulnerabilities here. I'm going
to need much more information in order to contribute to the discussion in an
informed manner.
As I told Alan in a separate thread (I wish we had kept this in one thread):
If applications want to change their behavior based on the caller, let them! Is
it bad practice, bad design, and likely all kinds of dumb? Heck yea. But there
are legitimate uses of this. Just because there are bad uses of this feature
doesn't mean you must omit it. If that's the case, then we need to get rid of
all input/output in Java--it could be used to write viruses to the file system!
Oh, and we should remove Random, too, because applications might Randomly
change their behavior! We must protect people from their own mistakes.
My sarcasm aside, I hope the point is clear. There are also legitimate uses for
things for which there are illegitimate uses. Farmers need diesel for their
tractors and fertilizer for their fields, but mix them and you can create a
bomb. A caller-sensitive API, discouraged as it may be, is sometimes 100%
necessary, and logging frameworks are the prime example. I also said in the
other thread:
Think of the performance improvements that could be had if, while determining
the source of an event, loggers could get the exact one frame they needed (via
StackTraceFrame#getCallerFrame, ~100ms per 1,000,000 calls) instead of having
to generate an entire stack trace and loop through it to find the one frame
(Thread#getStackTrace(), ~3,000ms per 1,000,000 calls).
I hope this is all articulated well. We need a caller-sensitive API. That's
just the whole of the story. Why should java.util.logging.Logger get to use
getCallerClass and Log4j not get to use it!? That is neither open nor fair.
Defining a SE supported @CallerSensitive and also getCallerClass API poses the risk of
"encouraging" developers to implement more @CS methods while not fully
understand its implication.
And that would be their mistake. Document the heck out of it! Put big red
warnings on it. Whatever makes you feel that you have disclaimed the user
enough, do it. Providing a file access API poses the risk of encouraging
developers to read and write files while not fully understanding the potential
security issues, too.
I'm just going to point out something, again, that I pointed out twice in June.
C#/.NET has the ability to A) get the caller Type (equiv. of our class) and B)
get the stack trace as StackFrames (the equivalent of the StackTraceFrame I'm
adding), which contain the Type. In fact, you can ONLY get the stack trace as
StackFrames. There's no StackTraceElement equivalent in C#/.NET. Has the world
blown up yet?
It was a non-goal of JEP 176 to provide @CallerSensitive as a public API and
I would suggest not to define it as a public API in JDK 8.
And, has been stated many, many times, this non-goal is incompatible with the
community's needs. Now, there /is/ a way to avoid making @CallerSensitive
public (which the community doesn't care about) while still making
getCallerClass public (which is really what the community needs). In order to
do so, you must remove the check that requires the method calling
getCallerClass/getCallerFrame to be annotated with @CallerSensitive. Once you
remove that check, you don't need @CallerSensitive to be public. To be clear,
though, once you remove that check, you don't need @CallerSensitive /at all/.
It can simply go away.
While I'll take the time to look at your patch, I would like to restart the
discussion from the use cases (in which led to what you summarize
Review request for 8021773: print function as defined by jrunscript's init.js script is incompatible with nashorn's definition
Bug: 8021773: print function as defined by jrunscript's init.js script is incompatible with nashorn's definition Please review http://cr.openjdk.java.net/~sundar/8021773/ Thanks -Sundar
Review request for 7187144: JavaDoc for ScriptEngineFactory.getProgram() contains an error
Bug: http://bugs.sun.com/view_bug.do?bug_id=7187144 Please review http://cr.openjdk.java.net/~sundar/7187144/ Thanks -Sundar
Re: Please review changes for JDK-8012975: Remove rhino from jdk8
I agree that it is better to remove com.sun.script.util package as well. It is not used elsewhere and was never declared as supported API. I've removed the same and re-generated webrev. http://cr.openjdk.java.net/~sundar/8012975/webrev.03/ I ran NEWBUILD=false as well as NEWBUILD=true to make sure build finishes fine. Please review. Thanks -Sundar On Monday 13 May 2013 05:56 PM, Alan Bateman wrote: On 13/05/2013 13:14, A. Sundararajan wrote: Incorporated changes as suggested. Uploaded webrev for historical purpose: http://cr.openjdk.java.net/~sundar/8012975/webrev.02/ PS. I am going ahead with push.. Thanks for fixing up refs.allowed, that one looks fine okay. Did you decide to keep com.sun.script.util? Just wondering because it appears that only com.sun.script.javascript is being removed. If the util API is going away then I assume that make/com/sun/script/Makefile should be removed (although we don't really care about the old build anymore). On the other, if the util API is staying then we need to figure out which profile is should go in. If nothing is using it then I assume it should be listed in FULL_JRE_RTJAR_INCLUDE_PACKAGES. -Alan
Re: Please review changes for JDK-8012975: Remove rhino from jdk8
Incorporated changes as suggested. Uploaded webrev for historical purpose: http://cr.openjdk.java.net/~sundar/8012975/webrev.02/ PS. I am going ahead with push.. Thanks -Sundar On Friday 10 May 2013 06:17 PM, A. Sundararajan wrote: Okay, thanks. com.sun.script.util is not supported API (no CCC done for it in the past). I'll remove it as suggested and run "make profiles" to check Thanks -Sundar On Friday 10 May 2013 04:09 PM, Alan Bateman wrote: On 10/05/2013 11:23, A. Sundararajan wrote: com/sun/script/util is generic utility package for script engine implementations. Only com/sun/script/javascript package is being removed. Since we include javax/script for profile 3, should we also include com/sun/script/util ? Is com.sun.script.util meant to be a supported/documented API? Do you know if anything outside of the JDK is using it? Is Nashorn using it? The only usage I see is in com.sun.script.javascript so this is why I assumed that com.sun.script.** would go away. As you know, we moved javax.script to compact1. It doesn't require com.sun.script.util of course but if this is used by 3rd party scripting engines then it may have to be added to compact1 builds to get them working. On refs.allowed, I'll remove it. How should I check this? "make profiles" on Linux should be fine. As part of the profiles build it will run a dependency analyzer that checks for references to types that do not exist (this is catch configuration issues). -Alan
Re: Please review changes for JDK-8012975: Remove rhino from jdk8
Okay, thanks. com.sun.script.util is not supported API (no CCC done for it in the past). I'll remove it as suggested and run "make profiles" to check Thanks -Sundar On Friday 10 May 2013 04:09 PM, Alan Bateman wrote: On 10/05/2013 11:23, A. Sundararajan wrote: com/sun/script/util is generic utility package for script engine implementations. Only com/sun/script/javascript package is being removed. Since we include javax/script for profile 3, should we also include com/sun/script/util ? Is com.sun.script.util meant to be a supported/documented API? Do you know if anything outside of the JDK is using it? Is Nashorn using it? The only usage I see is in com.sun.script.javascript so this is why I assumed that com.sun.script.** would go away. As you know, we moved javax.script to compact1. It doesn't require com.sun.script.util of course but if this is used by 3rd party scripting engines then it may have to be added to compact1 builds to get them working. On refs.allowed, I'll remove it. How should I check this? "make profiles" on Linux should be fine. As part of the profiles build it will run a dependency analyzer that checks for references to types that do not exist (this is catch configuration issues). -Alan
Re: Please review changes for JDK-8012975: Remove rhino from jdk8
com/sun/script/util is generic utility package for script engine implementations. Only com/sun/script/javascript package is being removed. Since we include javax/script for profile 3, should we also include com/sun/script/util ? On refs.allowed, I'll remove it. How should I check this? Thanks -Sundar On Friday 10 May 2013 03:03 PM, Alan Bateman wrote: On 10/05/2013 10:26, A. Sundararajan wrote: Please review the updated webrev @ http://cr.openjdk.java.net/~sundar/8012975/webrev.01/ Thanks -Sundar PROFILE_3_RTJAR_INCLUDE_PACKAGES needs to have com/sun/script removed. refs.allowed isn't quite right, the last line needs to be removed completely. Otherwise looks okay to me. -Alan.
Re: Please review changes for JDK-8012975: Remove rhino from jdk8
Please review the updated webrev @ http://cr.openjdk.java.net/~sundar/8012975/webrev.01/ Thanks -Sundar On Friday 03 May 2013 02:56 PM, Alan Bateman wrote: On 03/05/2013 07:47, A. Sundararajan wrote: Thanks. Looks like the first one has not been removed. But second one was removed: hg stat shows R make/sun/org/mozilla/javascript/Makefile (also the webrev shows it as removed). Perhaps patch does not take care of deleted files?? I am not sure. Also, build seems to go through without removing first one!! I'll remove that, build/test again and send another webrev. One other one is make/tools/src/build/tools/deps/refs.allowed. The last two lines can be replaced with: java.beans.PropertyChangeListener=java.util.logging.LogManager,compact1,compact2,compact3 and the comment line about Rhino can be removed. -Alan.
Re: Please review changes for JDK-8012975: Remove rhino from jdk8
On Friday 03 May 2013 11:53 AM, Tim Bell wrote: On 05/ 2/13 01:24 PM, I wrote: Hi Sundar: Oracle JDK includes Rhino based javax.script implementation (which lives mostly in "closed" code). Rhino is being removed from Oracle JDK builds and there are the changes to the jdk open repository as well like com.sun.script.javascript package, makefiles etc. Please review the open jdk changes here: http://cr.openjdk.java.net/~sundar/8012975/ This looks good. Approved. Tim Sundar - we have had some breakage in the build forest recently, so to be extra careful I created a forest and then added your changes. I also did some blasting away with 'find ... -print | xargs egrep ...' commands to look for traces of rhino or javascript. I think you need to look at removing these files as well: jdk/make/com/sun/script/Makefile jdk/make/sun/org/mozilla/javascript/Makefile Tim Thanks. Looks like the first one has not been removed. But second one was removed: hg stat shows R make/sun/org/mozilla/javascript/Makefile (also the webrev shows it as removed). Perhaps patch does not take care of deleted files?? I am not sure. Also, build seems to go through without removing first one!! I'll remove that, build/test again and send another webrev. Thanks -Sundar
Please review changes for JDK-8012975: Remove rhino from jdk8
Hi, Oracle JDK includes Rhino based javax.script implementation (which lives mostly in "closed" code). Rhino is being removed from Oracle JDK builds and there are the changes to the jdk open repository as well like com.sun.script.javascript package, makefiles etc. Please review the open jdk changes here: http://cr.openjdk.java.net/~sundar/8012975/ Thanks, -Sundar
Re: RFR: JDK-8013225: Refresh jdk's private ASM to the latest.
Hi Kumar, So long as those nashorn tests (jtreg tests under $jdk/test/javax/script, $jdk/sun/tools/jrunscript, $nashorn/test and nashorn ant tests - $nashorn/make - ant test) run fine, we've no objections from nashorn team. Thanks -Sundar On Tuesday 30 April 2013 03:55 AM, Kumar Srinivasan wrote: The restyling changes obfustucated things a bit but I didn't see anything of concern in casual review. I had hoped to see the updated SmallSet that didn't try to implement Iterator directly. It looks the SmallSet needs more discussion.. barring that anyone else have any other concerns with this change ? If I don't hear any objections I will push this on 05/01. Thanks Kumar Looks OK. The testing, which you have done, is the important qualifier for this change. Mike On Apr 25 2013, at 13:24 , Kumar Srinivasan wrote: Here is the webrev: http://cr.openjdk.java.net/~ksrini/8013225/webrev.0/ Thanks Kumar Hello, Please review changes which essentially contains asm5_future in asm's mainline repository, I have tested this patch with Nashorn (so has Sundar), as well as Lambda's usage. Fixes that I know of: 1. supports v52.0 class files and JSR 308/Type Annotations changes Thanks to Remi 2. elimination of "_" usages in variable names 3. several javadoc changes and one reported by Sundar http://jbs.oracle.com/bugs/browse/JDK-8010083 Thanks Kumar
Please review fix for JDK-8010083: Fix ASM doc comments to avoid javadoc errors
Please review http://cr.openjdk.java.net/~sundar/8010083/ Thanks -Sundar
Please review 8009140: jtreg tests under sun/tools/jrunscript should use nashorn engine
Hi, Please review http://cr.openjdk.java.net/~sundar/8009140/ Thanks -Sundar
Re: Review request for 8009115: jtreg tests under jdk/test/javax/script should use nashorn as script engine
Yes, that is the plan (i.e., removal of rhino in Oracle builds). I am
looking at jrunscript tests too.
Thanks
-Sundar
On Wednesday 27 February 2013 05:52 PM, Alan Bateman wrote:
On 27/02/2013 11:20, A. Sundararajan wrote:
Thanks Alan. Yes, nashorn is a "js" engine as well and so user's java
code can use getEngineByName("js") and get it. The current change is
to make sure that javax.script tests use nashorn engine - rather than
any other "js" engine that may be present in the underlying jdk.
-Sundar
I assume Rhino in the Oracle builds should now be removed or disabled.
The changes are fine for today of course. It would be good to look at
the tests in sun/tools/jrunscript too as I I think there may be
changes required there too.
-Alan
Re: Review request for 8009115: jtreg tests under jdk/test/javax/script should use nashorn as script engine
Thanks Alan. Yes, nashorn is a "js" engine as well and so user's java
code can use getEngineByName("js") and get it. The current change is to
make sure that javax.script tests use nashorn engine - rather than any
other "js" engine that may be present in the underlying jdk.
-Sundar
On Wednesday 27 February 2013 04:47 PM, Alan Bateman wrote:
On 27/02/2013 10:44, A. Sundararajan wrote:
Hi,
Please review http://cr.openjdk.java.net/~sundar/8009115/
Changing javax.script tests to use nashorn engine explicitly.
Adjusted tests for differences b/w nashorn and rhino engines.
Changes documented here:
http://cr.openjdk.java.net/~sundar/8009115/README
Thanks
-Sundar
These changes look okay to me although I assumed that Nashorn would be
a "js" scripting engine, in which case there shouldn't be a need to
change many of the getEngineByName("js") usages to "nashorn".
-Alan
Review request for 8009115: jtreg tests under jdk/test/javax/script should use nashorn as script engine
Hi, Please review http://cr.openjdk.java.net/~sundar/8009115/ Changing javax.script tests to use nashorn engine explicitly. Adjusted tests for differences b/w nashorn and rhino engines. Changes documented here: http://cr.openjdk.java.net/~sundar/8009115/README Thanks -Sundar
Codereview request for 8009115: jtreg tests under jdk/test/javax/script should use nashorn as script engine
Please review http://cr.openjdk.java.net/~sundar/8009115/ Changing javax.script tests to use nashorn engine explicitly - also adjusting tests to reflect (minor) changes b/w nashorn and rhino. Thanks -Sundar
Re: 7127687: MethodType leaks memory due to interning
Looks good to me. PS. Remi notes that only constructor and "add" method of WeakInternSet are accessed from the containing class. The rest can be made private. -Sundar John Rose wrote: Thanks, Jim. -- John (on my iPhone T-1000) On Mar 28, 2012, at 6:01 PM, Jim Laskey wrote: The WeakHashMap leads to a non-weak reference to the class, since only the key is weak. Same is true for public versions of WeakHashSet. The collection used here is truly weak. Sent from my iPhone 4 On 2012-03-28, at 9:42 PM, Vitaly Davidovich wrote: Hi John, I think you can use diamond generic inference when declaring the weak intern set. Also any reason you didn't use WeakHashMap directly with dummy value to simulate the set? Or wrap the WeakHashMap and synchronize the accessors to it? Sent from my phone On Mar 28, 2012 7:52 PM, "John Rose" wrote: http://cr.openjdk.java.net/~jrose/7127687/webrev.00/ 7127687: MethodType leaks memory due to interning Summary: Replace internTable with a weak-reference version. This is a point fix for JDK 8, and will (pending approval) also be back-ported to JDK 7u. — John Notes on process: This code is part of JSR 292. Therefore the review comments will be collected in mlvm-dev, and changes will be integrated via hsx/hotspot-comp. At least one reviewer must be an official Reviewer the JDK 8 Project [1], but other reviewers are most welcome. [1] http://openjdk.java.net/census#jdk8 ___ mlvm-dev mailing list mlvm-...@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev ___ mlvm-dev mailing list mlvm-...@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev ___ mlvm-dev mailing list mlvm-...@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
