uschindler commented on code in PR #12873:
URL: https://github.com/apache/lucene/pull/12873#discussion_r1414718350
##########
lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java:
##########
@@ -201,48 +226,57 @@ private static void unusedTestCompile() throws
IOException {
*
* @param sourceText The expression to compile
*/
- private JavascriptCompiler(String sourceText, Map<String, Method> functions,
boolean picky) {
- if (sourceText == null) {
- throw new NullPointerException();
- }
- this.sourceText = sourceText;
- this.functions = functions;
+ private JavascriptCompiler(
+ String sourceText, Map<String, MethodHandle> functions, boolean picky) {
+ this.sourceText = Objects.requireNonNull(sourceText, "sourceText");
+ this.functions = Map.copyOf(functions);
this.picky = picky;
}
/**
- * Compiles the given expression with the specified parent classloader
+ * Compiles the given expression as hidden class.
*
* @return A new compiled expression
* @throws ParseException on failure to compile
*/
- private Expression compileExpression(ClassLoader parent) throws
ParseException {
+ private Expression compileExpression() throws ParseException {
final Map<String, Integer> externalsMap = new LinkedHashMap<>();
final ClassWriter classWriter =
new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS);
try {
generateClass(getAntlrParseTree(), classWriter, externalsMap);
-
- final Class<? extends Expression> evaluatorClass =
- new Loader(parent).define(COMPILED_EXPRESSION_CLASS,
classWriter.toByteArray());
- final Constructor<? extends Expression> constructor =
- evaluatorClass.getConstructor(String.class, String[].class);
-
- return constructor.newInstance(
- sourceText, externalsMap.keySet().toArray(new
String[externalsMap.size()]));
} catch (RuntimeException re) {
if (re.getCause() instanceof ParseException) {
throw (ParseException) re.getCause();
}
throw re;
+ }
+
+ try {
+ final Lookup lookup =
+ LOOKUP.defineHiddenClassWithClassData(classWriter.toByteArray(),
functions, true);
+ return invokeConstructor(lookup, lookup.lookupClass(), externalsMap);
} catch (ReflectiveOperationException exception) {
throw new IllegalStateException(
"An internal error occurred attempting to compile the expression ("
+ sourceText + ").",
exception);
}
}
+ private Expression invokeConstructor(
+ Lookup lookup, Class<?> expressionClass, Map<String, Integer>
externalsMap)
+ throws ReflectiveOperationException {
+ final MethodHandle ctor = lookup.findConstructor(expressionClass,
MT_EXPRESSION_CTOR_LOOKUP);
+ try {
+ return (Expression) ctor.invoke(sourceText,
externalsMap.keySet().toArray(String[]::new));
Review Comment:
this is a one-time call and manually adapting makes no sense here. We have
more of those in Lucene when we call constructors for singletons (see provider
interfaces for vectors and MMap).
You can adapt it, but this is not worth it. There is a reason we have no
forbiddenapis.
P.S.: The adaption is needed because of return type. A constructor returns
its own class, which cannot be assigned without adaption to a superclass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
