From 234a2c4d9cd6273ec10e6a54a492440fd6d739bd Mon Sep 17 00:00:00 2001 From: Kaz Date: Fri, 20 Sep 2024 12:45:01 -0700 Subject: [PATCH] Stateless parser API Stateless (static) parser interface. Buffer-reuse optimization is now hidden behind JNI FFI implementation. Fixes #11121 and prevents similar bugs. --- .../org/enso/runner/EnsoLibraryFeature.java | 5 +- .../scala/org/enso/compiler/Compiler.scala | 14 ++-- .../compiler/phase/BuiltinsIrBuilder.scala | 6 +- .../instrument/ChangesetBuilder.scala | 15 ++-- .../compiler/test/CompilerTestSetup.scala | 42 +--------- .../org/enso/compiler/test/CompilerTests.java | 17 +--- .../test/VectorArraySignatureTest.java | 21 +---- .../org/enso/compiler/test/CompilerTest.scala | 8 +- .../org/enso/compiler/core/EnsoParser.java | 49 +++--------- .../enso/compiler/core/EnsoParserTest.java | 28 +------ .../enso/ydoc/polyfill/ParserPolyfill.java | 23 ++---- .../java/org/enso/syntax2/Parser.java | 80 +++++++++---------- .../generate-java/src/bin/java-tests.rs | 4 +- lib/rust/parser/jni/src/lib.rs | 68 ++++------------ .../enso4igv/enso/EnsoErrorProvider.java | 3 +- .../tools/enso4igv/enso/EnsoStructure.java | 3 +- 16 files changed, 100 insertions(+), 286 deletions(-) diff --git a/engine/runner/src/main/java/org/enso/runner/EnsoLibraryFeature.java b/engine/runner/src/main/java/org/enso/runner/EnsoLibraryFeature.java index 3463df945e2b4..433271da46396 100644 --- a/engine/runner/src/main/java/org/enso/runner/EnsoLibraryFeature.java +++ b/engine/runner/src/main/java/org/enso/runner/EnsoLibraryFeature.java @@ -6,6 +6,7 @@ import java.nio.file.Path; import java.util.LinkedHashSet; import java.util.TreeSet; +import org.enso.compiler.core.EnsoParser; import org.enso.compiler.core.ir.module.scope.imports.Polyglot; import org.enso.pkg.PackageManager$; import org.graalvm.nativeimage.hosted.Feature; @@ -45,14 +46,14 @@ public void beforeAnalysis(BeforeAnalysisAccess access) { */ var classes = new TreeSet(); - try (var parser = new org.enso.compiler.core.EnsoParser()) { + try { for (var p : libs) { var result = PackageManager$.MODULE$.Default().loadPackage(p.toFile()); if (result.isSuccess()) { var pkg = result.get(); for (var src : pkg.listSourcesJava()) { var code = Files.readString(src.file().toPath()); - var ir = parser.compile(code); + var ir = EnsoParser.compile(code); for (var imp : asJava(ir.imports())) { if (imp instanceof Polyglot poly && poly.entity() instanceof Polyglot.Java entity) { var name = new StringBuilder(entity.getJavaName()); diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/Compiler.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/Compiler.scala index 95d96e71399d0..76c70fc453d63 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/Compiler.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/Compiler.scala @@ -35,6 +35,7 @@ import org.enso.compiler.phase.exports.{ ExportsResolution } import org.enso.syntax2.Tree +import org.enso.syntax2.Parser import java.io.PrintStream import java.util.concurrent.{ @@ -69,7 +70,6 @@ class Compiler( if (config.outputRedirect.isDefined) new PrintStream(config.outputRedirect.get) else context.getOut - private lazy val ensoCompiler: EnsoParser = new EnsoParser() /** Java accessor */ def getConfig(): CompilerConfig = config @@ -598,11 +598,8 @@ class Compiler( ) val src = context.getCharacters(module) - val idMap = context.getIdMap(module) - val tree = ensoCompiler.parse(src) - val expr = - if (idMap == null) ensoCompiler.generateIR(tree) - else ensoCompiler.generateModuleIr(tree, idMap.values) + val idMap = Option(context.getIdMap(module)) + val expr = EnsoParser.compile(src, idMap.map(_.values).orNull) val exprWithModuleExports = if (context.isSynthetic(module)) @@ -685,9 +682,8 @@ class Compiler( inlineContext: InlineContext ): Option[(InlineContext, Expression)] = { val newContext = inlineContext.copy(freshNameSupply = Some(freshNameSupply)) - val tree = ensoCompiler.parse(srcString) - ensoCompiler.generateIRInline(tree).map { ir => + EnsoParser.compileInline(srcString).map { ir => val compilerOutput = runCompilerPhasesInline(ir, newContext) runErrorHandlingInline(compilerOutput, newContext) (newContext, compilerOutput) @@ -700,7 +696,7 @@ class Compiler( * @return A Tree representation of `source` */ def parseInline(source: CharSequence): Tree = - ensoCompiler.parse(source) + Parser.parse(source) /** Enhances the provided IR with import/export statements for the provided list * of fully qualified names of modules. The statements are considered to be "synthetic" i.e. compiler-generated. diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/phase/BuiltinsIrBuilder.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/phase/BuiltinsIrBuilder.scala index 255aa98a2f992..2f1785d2f8984 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/phase/BuiltinsIrBuilder.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/phase/BuiltinsIrBuilder.scala @@ -11,8 +11,6 @@ import org.enso.compiler.data.CompilerConfig import org.enso.common.CompilationStage import org.enso.compiler.phase.exports.ExportsResolution -import scala.util.Using - /** A phase responsible for initializing the builtins' IR from the provided * source. */ @@ -44,9 +42,7 @@ object BuiltinsIrBuilder { freshNameSupply = Some(freshNameSupply), compilerConfig = CompilerConfig(warningsEnabled = false) ) - val initialIr = Using(new EnsoParser) { compiler => - compiler.compile(module.getCharacters) - }.get + val initialIr = EnsoParser.compile(module.getCharacters) val irAfterModDiscovery = passManager.runPassesOnModule( initialIr, moduleContext, diff --git a/engine/runtime-instrument-common/src/main/scala/org/enso/interpreter/instrument/ChangesetBuilder.scala b/engine/runtime-instrument-common/src/main/scala/org/enso/interpreter/instrument/ChangesetBuilder.scala index 7ad527e5dafcc..c872efd9fe716 100644 --- a/engine/runtime-instrument-common/src/main/scala/org/enso/interpreter/instrument/ChangesetBuilder.scala +++ b/engine/runtime-instrument-common/src/main/scala/org/enso/interpreter/instrument/ChangesetBuilder.scala @@ -13,7 +13,6 @@ import org.enso.text.editing.{IndexedSource, TextEditor} import java.util.UUID import scala.collection.mutable -import scala.util.Using /** The changeset of a module containing the computed list of invalidated * expressions. @@ -97,14 +96,12 @@ final class ChangesetBuilder[A: TextEditor: IndexedSource]( } val source = Source.newBuilder("enso", value, null).build - Using(new EnsoParser) { compiler => - compiler - .generateIRInline(compiler.parse(source.getCharacters())) - .flatMap(_ match { - case ir: Literal => Some(ir.setLocation(oldIr.location)) - case _ => None - }) - }.get + EnsoParser + .compileInline(source.getCharacters()) + .flatMap(_ match { + case ir: Literal => Some(ir.setLocation(oldIr.location)) + case _ => None + }) } oldIr match { diff --git a/engine/runtime-instrument-common/src/test/scala/org/enso/compiler/test/CompilerTestSetup.scala b/engine/runtime-instrument-common/src/test/scala/org/enso/compiler/test/CompilerTestSetup.scala index 17b353bd464ec..c7fbe852fc661 100644 --- a/engine/runtime-instrument-common/src/test/scala/org/enso/compiler/test/CompilerTestSetup.scala +++ b/engine/runtime-instrument-common/src/test/scala/org/enso/compiler/test/CompilerTestSetup.scala @@ -21,42 +21,6 @@ import org.enso.common.CompilationStage trait CompilerTestSetup { // === IR Utilities ========================================================= - /** An extension method to allow converting string source code to IR as a - * module. - * - * @param source the source code to convert - */ - implicit private class ToIrModule(source: String) { - - /** Converts program text to a top-level Enso module. - * - * @return the [[IR]] representing [[source]] - */ - def toIrModule: Module = { - val compiler = new EnsoParser() - try compiler.compile(source) - finally compiler.close() - } - } - - /** An extension method to allow converting string source code to IR as an - * expression. - * - * @param source the source code to convert - */ - implicit private class ToIrExpression(source: String) { - - /** Converts the program text to an Enso expression. - * - * @return the [[IR]] representing [[source]], if it is a valid expression - */ - def toIrExpression: Option[Expression] = { - val compiler = new EnsoParser() - try compiler.generateIRInline(compiler.parse(source)) - finally compiler.close() - } - } - /** Provides an extension method allowing the running of a specified list of * passes on the provided IR. * @@ -112,7 +76,7 @@ trait CompilerTestSetup { * @return IR appropriate for testing the alias analysis pass as a module */ def preprocessModule(implicit moduleContext: ModuleContext): Module = { - source.toIrModule.runPasses(passManager, moduleContext) + EnsoParser.compile(source).runPasses(passManager, moduleContext) } /** Translates the source code into appropriate IR for testing this pass @@ -123,7 +87,9 @@ trait CompilerTestSetup { def preprocessExpression(implicit inlineContext: InlineContext ): Option[Expression] = { - source.toIrExpression.map(_.runPasses(passManager, inlineContext)) + EnsoParser + .compileInline(source) + .map(_.runPasses(passManager, inlineContext)) } } diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/CompilerTests.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/CompilerTests.java index 7b01bed83311e..3a3affc082ba7 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/CompilerTests.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/CompilerTests.java @@ -11,25 +11,10 @@ import org.enso.compiler.core.EnsoParser; import org.enso.compiler.core.IR; import org.enso.compiler.core.ir.Module; -import org.junit.AfterClass; -import org.junit.BeforeClass; public abstract class CompilerTests { - - protected static EnsoParser ensoCompiler; - - @BeforeClass - public static void initEnsoParser() { - ensoCompiler = new EnsoParser(); - } - - @AfterClass - public static void closeEnsoParser() throws Exception { - ensoCompiler.close(); - } - protected static Module parse(CharSequence code) { - Module ir = ensoCompiler.compile(code); + Module ir = EnsoParser.compile(code); assertNotNull("IR was generated", ir); return ir; } diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/VectorArraySignatureTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/VectorArraySignatureTest.java index 5cabcd4cb387f..e8f03c272562a 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/VectorArraySignatureTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/VectorArraySignatureTest.java @@ -19,25 +19,10 @@ import org.enso.compiler.core.ir.Name; import org.enso.compiler.core.ir.expression.Comment; import org.enso.compiler.core.ir.module.scope.Definition; -import org.junit.AfterClass; -import org.junit.BeforeClass; import org.junit.Test; import scala.Function1; public class VectorArraySignatureTest { - private static EnsoParser ensoCompiler; - - @BeforeClass - public static void initEnsoParser() { - ensoCompiler = new EnsoParser(); - } - - @AfterClass - public static void closeEnsoParser() throws Exception { - ensoCompiler.close(); - ensoCompiler = null; - } - @Test public void testParseVectorAndArray() throws Exception { var p = Paths.get("../../distribution/").toFile().getCanonicalFile(); @@ -81,8 +66,7 @@ public FileVisitResult postVisitDirectory(Path t, IOException ioe) throws IOExce var vectorSrc = Files.readString(vectorAndArray[1]); var arrayIR = - ensoCompiler - .compile(arraySrc) + EnsoParser.compile(arraySrc) .preorder() .filter( (v) -> { @@ -95,8 +79,7 @@ public FileVisitResult postVisitDirectory(Path t, IOException ioe) throws IOExce }) .head(); var vectorIR = - ensoCompiler - .compile(vectorSrc) + EnsoParser.compile(vectorSrc) .preorder() .filter( (v) -> { diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala index 7befe14b968a9..a25192c6b0657 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala @@ -43,9 +43,7 @@ trait CompilerRunner { * @return the [[IR]] representing [[source]] */ def toIrModule: Module = { - val compiler = new EnsoParser() - try compiler.compile(source) - finally compiler.close() + EnsoParser.compile(source) } } @@ -61,9 +59,7 @@ trait CompilerRunner { * @return the [[IR]] representing [[source]], if it is a valid expression */ def toIrExpression: Option[Expression] = { - val compiler = new EnsoParser() - try compiler.generateIRInline(compiler.parse(source)) - finally compiler.close() + EnsoParser.compileInline(source) } } diff --git a/engine/runtime-parser/src/main/java/org/enso/compiler/core/EnsoParser.java b/engine/runtime-parser/src/main/java/org/enso/compiler/core/EnsoParser.java index a44cdac9b9ec1..2b0a63efdd234 100644 --- a/engine/runtime-parser/src/main/java/org/enso/compiler/core/EnsoParser.java +++ b/engine/runtime-parser/src/main/java/org/enso/compiler/core/EnsoParser.java @@ -6,48 +6,23 @@ import org.enso.compiler.core.ir.Location; import org.enso.compiler.core.ir.Module; import org.enso.syntax2.Parser; -import org.enso.syntax2.Tree; -public final class EnsoParser implements AutoCloseable { - private final Parser parser; - - public EnsoParser() { - Parser p; - try { - p = Parser.create(); - } catch (LinkageError err) { - err.printStackTrace(); - throw err; - } - this.parser = p; +public final class EnsoParser { + public static Module compile(CharSequence src) { + return compile(src, null); } - @Override - public void close() throws Exception { - if (parser != null) { - parser.close(); + public static Module compile(CharSequence src, Map idMap) { + var tree = Parser.parse(src); + var treeToIr = TreeToIr.MODULE; + if (idMap != null) { + treeToIr = new TreeToIr(idMap); } + return treeToIr.translate(tree); } - public Module compile(CharSequence src) { - var tree = parser.parse(src); - return generateIR(tree); - } - - public Tree parse(CharSequence src) { - return parser.parse(src); - } - - public Module generateIR(Tree t) { - return TreeToIr.MODULE.translate(t); - } - - public Module generateModuleIr(Tree t, Map idMap) { - var treeToIr = new TreeToIr(idMap); - return treeToIr.translate(t); - } - - public scala.Option generateIRInline(Tree t) { - return TreeToIr.MODULE.translateInline(t); + public static scala.Option compileInline(CharSequence src) { + var tree = Parser.parse(src); + return TreeToIr.MODULE.translateInline(tree); } } diff --git a/engine/runtime-parser/src/test/java/org/enso/compiler/core/EnsoParserTest.java b/engine/runtime-parser/src/test/java/org/enso/compiler/core/EnsoParserTest.java index 249151bec5e23..256d415952398 100644 --- a/engine/runtime-parser/src/test/java/org/enso/compiler/core/EnsoParserTest.java +++ b/engine/runtime-parser/src/test/java/org/enso/compiler/core/EnsoParserTest.java @@ -19,28 +19,10 @@ import org.enso.compiler.core.ir.Module; import org.enso.compiler.core.ir.expression.Error; import org.enso.compiler.core.ir.module.scope.definition.Method; -import org.junit.AfterClass; -import org.junit.BeforeClass; import org.junit.Test; import scala.jdk.javaapi.CollectionConverters; public class EnsoParserTest { - private static EnsoParser ensoCompiler; - - @BeforeClass - public static void initEnsoParser() { - try { - ensoCompiler = new EnsoParser(); - } catch (LinkageError e) { - throw new AssertionError(e); - } - } - - @AfterClass - public static void closeEnsoParser() throws Exception { - if (ensoCompiler != null) ensoCompiler.close(); - } - @Test public void testParseMain7Foo() { parseTest(""" @@ -1529,7 +1511,9 @@ private static void equivalenceTest(String code1, String code2) throws IOExcepti } private static Module compile(String code) { - return compile(ensoCompiler, code); + var ir = EnsoParser.compile(code); + assertNotNull("IR was generated", ir); + return ir; } private void expectNoErrorsInIr(Module moduleIr) { @@ -1544,12 +1528,6 @@ private void expectNoErrorsInIr(Module moduleIr) { }); } - public static Module compile(EnsoParser c, String code) { - var ir = c.compile(code); - assertNotNull("IR was generated", ir); - return ir; - } - static void assertIR(String msg, Module old, Module now) throws IOException { Function filter = f -> simplifyIR(f, true, true, false); String ir1 = filter.apply(old); diff --git a/lib/java/ydoc-server/src/main/java/org/enso/ydoc/polyfill/ParserPolyfill.java b/lib/java/ydoc-server/src/main/java/org/enso/ydoc/polyfill/ParserPolyfill.java index 5cc989cfa27ca..f93dfa4a17de7 100644 --- a/lib/java/ydoc-server/src/main/java/org/enso/ydoc/polyfill/ParserPolyfill.java +++ b/lib/java/ydoc-server/src/main/java/org/enso/ydoc/polyfill/ParserPolyfill.java @@ -19,21 +19,10 @@ public final class ParserPolyfill implements AutoCloseable, ProxyExecutable, Pol private static final String PARSER_JS = "parser.js"; - private final Parser parser; - - public ParserPolyfill() { - Parser p; - try { - p = Parser.create(); - } catch (LinkageError e) { - log.error("Failed to create parser", e); - throw e; - } - this.parser = p; - } + public ParserPolyfill() {} @Override - public final void initialize(Context ctx) { + public void initialize(Context ctx) { Source parserJs = Source.newBuilder("js", ParserPolyfill.class.getResource(PARSER_JS)).buildLiteral(); @@ -50,7 +39,7 @@ public Object execute(Value... arguments) { case PARSE_TREE -> { var input = arguments[1].asString(); - yield parser.parseInputLazy(input); + yield Parser.parseInputLazy(input); } case XX_HASH_128 -> { @@ -62,7 +51,7 @@ public Object execute(Value... arguments) { case IS_IDENT_OR_OPERATOR -> { var input = arguments[1].asString(); - yield parser.isIdentOrOperator(input); + yield Parser.isIdentOrOperator(input); } default -> throw new IllegalStateException(command); @@ -70,7 +59,5 @@ public Object execute(Value... arguments) { } @Override - public void close() { - parser.close(); - } + public void close() {} } diff --git a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java index 91731cdde522c..191c0a59f1ec0 100644 --- a/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java +++ b/lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java @@ -5,8 +5,11 @@ import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.charset.StandardCharsets; +import java.util.concurrent.atomic.AtomicBoolean; + +public final class Parser { + static AtomicBoolean initialized = new AtomicBoolean(false); -public final class Parser implements AutoCloseable { private static void initializeLibraries() { try { System.loadLibrary("enso_parser"); @@ -39,7 +42,7 @@ private static void initializeLibraries() { if (path.exists()) break; d = d.getParentFile(); } - if (d == null || path == null) { + if (d == null) { throw new LinkageError("Cannot find parser in " + root); } System.load(path.getAbsolutePath()); @@ -55,6 +58,19 @@ e, new File(".").getAbsoluteFile(), "target", "rust", "parser-jni", name)) { } } + private static void ensureInitialized() { + // Optimization: Skip finding and initializing the library when it's definitely already been + // done. + // Note that this inexpensive check may produce false negatives: Nothing prevents multiple + // threads from proceeding + // to library initialization concurrently (before any thread has completed initialization), but + // that is harmless; + // `initializeLibraries` is thread-safe and idempotent. + if (initialized.get()) return; + initializeLibraries(); + initialized.set(true); + } + private static boolean searchFromDirToTop(Throwable chain, File root, String... names) { while (root != null) { var parser = root; @@ -75,25 +91,19 @@ private static boolean searchFromDirToTop(Throwable chain, File root, String... return false; } - private long stateUnlessClosed; + private Parser() {} - private Parser(long stateIn) { - stateUnlessClosed = stateIn; - } + private static native void freeBuffers(); - private static native long allocState(); + private static native ByteBuffer parseTree(ByteBuffer input); - private static native void freeState(long state); - - private static native ByteBuffer parseTree(long state, ByteBuffer input); - - private static native ByteBuffer parseTreeLazy(long state, ByteBuffer input); + private static native ByteBuffer parseTreeLazy(ByteBuffer input); private static native long isIdentOrOperator(ByteBuffer input); - private static native long getLastInputBase(long state); + private static native long getLastInputBase(); - private static native long getMetadata(long state); + private static native long getMetadata(); private static native String getWarningTemplate(int warningId); @@ -101,56 +111,38 @@ private Parser(long stateIn) { static native long getUuidLow(long metadata, long codeOffset, long codeLength); - public static Parser create() { - initializeLibraries(); - var state = allocState(); - return new Parser(state); - } - - public long isIdentOrOperator(CharSequence input) { + public static long isIdentOrOperator(CharSequence input) { + ensureInitialized(); byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); inputBuf.put(inputBytes); - return isIdentOrOperator(inputBuf); } - private long getState() { - if (stateUnlessClosed != 0) { - return stateUnlessClosed; - } else { - throw new IllegalStateException("Parser used after close()"); - } - } - - public ByteBuffer parseInputLazy(CharSequence input) { - var state = getState(); + public static ByteBuffer parseInputLazy(CharSequence input) { + ensureInitialized(); byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); inputBuf.put(inputBytes); - return parseTreeLazy(state, inputBuf); + return parseTreeLazy(inputBuf); } - public Tree parse(CharSequence input) { - var state = getState(); + public static Tree parse(CharSequence input) { + ensureInitialized(); byte[] inputBytes = input.toString().getBytes(StandardCharsets.UTF_8); ByteBuffer inputBuf = ByteBuffer.allocateDirect(inputBytes.length); inputBuf.put(inputBytes); - var serializedTree = parseTree(state, inputBuf); - var base = getLastInputBase(state); - var metadata = getMetadata(state); + var serializedTree = parseTree(inputBuf); + var base = getLastInputBase(); + var metadata = getMetadata(); serializedTree.order(ByteOrder.LITTLE_ENDIAN); var message = new Message(serializedTree, input, base, metadata); return Tree.deserialize(message); } public static String getWarningMessage(Warning warning) { + // `ensureInitialized` is unnecessary here because a `Warning` will only be constructed by + // parsing something. return getWarningTemplate(warning.getId()); } - - @Override - public void close() { - freeState(stateUnlessClosed); - stateUnlessClosed = 0; - } } diff --git a/lib/rust/parser/generate-java/src/bin/java-tests.rs b/lib/rust/parser/generate-java/src/bin/java-tests.rs index daf00cc87dd7d..49c551aee57ff 100644 --- a/lib/rust/parser/generate-java/src/bin/java-tests.rs +++ b/lib/rust/parser/generate-java/src/bin/java-tests.rs @@ -28,7 +28,9 @@ fn main() { println!("import java.nio.ByteOrder;"); println!(); println!("class GeneratedFormatTests {{"); - println!(" private static final Object INIT = {package}.Parser.create();"); + // Force the parser to load its shared library. `parse` handles this because usually it is the + // entry point to the class, but we're doing low-level operations directly. + println!(" private static final Object INIT = {package}.Parser.parse(\"\");"); println!(" private static java.util.Vector accept;"); println!(" private static java.util.Vector reject;"); for (i, case) in cases.accept.iter().enumerate() { diff --git a/lib/rust/parser/jni/src/lib.rs b/lib/rust/parser/jni/src/lib.rs index 0ee8580153b0b..b54b3255eafa1 100644 --- a/lib/rust/parser/jni/src/lib.rs +++ b/lib/rust/parser/jni/src/lib.rs @@ -16,6 +16,7 @@ use jni::objects::JClass; use jni::sys::jobject; use jni::sys::jstring; use jni::JNIEnv; +use std::cell::UnsafeCell; @@ -31,19 +32,15 @@ static FAILED_SERIALIZE_AST: &str = "Failed to serialize AST to binary format."; /// /// # Safety /// -/// The state MUST be a value returned by `allocState` that has not been passed to `freeState`. -/// The input buffer contents MUST be valid UTF-8. -/// The contents of the returned buffer MUST not be accessed after another call to `parseInput`, or -/// a call to `freeState`. +/// The contents of the returned buffer MUST not be accessed after another call to `parseInput`. #[allow(unsafe_code)] #[no_mangle] pub extern "system" fn Java_org_enso_syntax2_Parser_parseTree( mut env: JNIEnv, _class: JClass, - state: u64, input: JByteBuffer, ) -> jobject { - let state = unsafe { &mut *(state as usize as *mut State) }; + let state = STATE.with(|state| unsafe { state.get().as_mut() }).unwrap(); let input = unsafe { decode_utf8_buffer(&env, &input) }; let mut code = input; let mut meta = None; @@ -76,19 +73,16 @@ pub extern "system" fn Java_org_enso_syntax2_Parser_parseTree( /// /// # Safety /// -/// The state MUST be a value returned by `allocState` that has not been passed to `freeState`. /// The input buffer contents MUST be valid UTF-8. -/// The contents of the returned buffer MUST not be accessed after another call to `parseInput`, or -/// a call to `freeState`. +/// The contents of the returned buffer MUST not be accessed after another call to `parseInput`. #[allow(unsafe_code)] #[no_mangle] pub extern "system" fn Java_org_enso_syntax2_Parser_parseTreeLazy( mut env: JNIEnv, _class: JClass, - state: u64, input: JByteBuffer, ) -> jobject { - let state = unsafe { &mut *(state as usize as *mut State) }; + let state = STATE.with(|state| unsafe { state.get().as_mut() }).unwrap(); let input = unsafe { decode_utf8_buffer(&env, &input) }; let tree = enso_parser::Parser::new().run(input); @@ -126,37 +120,24 @@ pub extern "system" fn Java_org_enso_syntax2_Parser_isIdentOrOperator( /// Return the `base` parameter to pass to the `Message` class along with the other output of the /// most recent call to `parseInput`. -/// -/// # Safety -/// -/// The input MUST have been returned by `allocState`, and MUST NOT have previously been passed to -/// `freeState`. #[allow(unsafe_code)] #[no_mangle] pub extern "system" fn Java_org_enso_syntax2_Parser_getLastInputBase( _env: JNIEnv, _class: JClass, - state: u64, ) -> u64 { - let state = unsafe { &mut *(state as usize as *mut State) }; - state.base + STATE.with(|state| unsafe { state.get().as_ref() }).unwrap().base } /// Return the metadata associated with the most recent parse. -/// -/// # Safety -/// -/// The input MUST have been returned by `allocState`, and MUST NOT have previously been passed to -/// `freeState`. #[allow(unsafe_code)] #[no_mangle] pub extern "system" fn Java_org_enso_syntax2_Parser_getMetadata( _env: JNIEnv, _class: JClass, - state: u64, ) -> u64 { - let state = unsafe { &mut *(state as usize as *mut State) }; - match &state.metadata { + let metadata = &STATE.with(|state| unsafe { state.get().as_ref() }).unwrap().metadata; + match metadata { Some(metadata) => { let metadata: *const _ = metadata; metadata as usize as u64 @@ -165,34 +146,11 @@ pub extern "system" fn Java_org_enso_syntax2_Parser_getMetadata( } } -/// Allocate a new parser state object. The returned value should be passed to `freeState` when no -/// longer needed. -#[allow(unsafe_code, clippy::box_default)] -#[no_mangle] -pub extern "system" fn Java_org_enso_syntax2_Parser_allocState( - _env: JNIEnv, - _class: JClass, -) -> u64 { - Box::into_raw(Box::new(State::default())) as _ -} - -/// Free the resources owned by the state object. -/// -/// # Safety -/// -/// The input MUST have been returned by `allocState`, and MUST NOT have previously been passed to -/// `freeState`. +/// Free the thread's parsing resources. #[allow(unsafe_code)] #[no_mangle] -pub extern "system" fn Java_org_enso_syntax2_Parser_freeState( - _env: JNIEnv, - _class: JClass, - state: u64, -) { - if state != 0 { - let state = unsafe { Box::from_raw(state as usize as *mut State) }; - drop(state); - } +pub extern "system" fn Java_org_enso_syntax2_Parser_freeBuffers(_env: JNIEnv, _class: JClass) { + *STATE.with(|state| unsafe { state.get().as_mut() }).unwrap() = Default::default(); } /// Returns the string template corresponding to the given warning ID. @@ -294,3 +252,7 @@ struct State { output: Vec, metadata: Option, } + +thread_local! { + static STATE: UnsafeCell = Default::default(); +} diff --git a/tools/enso4igv/src/main/java/org/enso/tools/enso4igv/enso/EnsoErrorProvider.java b/tools/enso4igv/src/main/java/org/enso/tools/enso4igv/enso/EnsoErrorProvider.java index 813107e90095b..e166a9313ad20 100644 --- a/tools/enso4igv/src/main/java/org/enso/tools/enso4igv/enso/EnsoErrorProvider.java +++ b/tools/enso4igv/src/main/java/org/enso/tools/enso4igv/enso/EnsoErrorProvider.java @@ -27,12 +27,11 @@ public List computeErrors(Context ctx) { try { if (ctx.errorKind() == Kind.ERRORS) { LOG.log(Level.FINE, "Processing errors for {0}", ctx.file().getPath()); - var parser = new EnsoParser(); var text = toText(ctx); Function1 where = (loc) -> { return text.substring(loc.start(), loc.end()); }; - var moduleIr = parser.compile(text); + var moduleIr = EnsoParser.compile(text); moduleIr.preorder().foreach((p) -> { if (p instanceof Syntax err && err.location().isDefined()) { var loc = err.location().get(); diff --git a/tools/enso4igv/src/main/java/org/enso/tools/enso4igv/enso/EnsoStructure.java b/tools/enso4igv/src/main/java/org/enso/tools/enso4igv/enso/EnsoStructure.java index 13e2d169cceb0..6094b5f9c4e66 100644 --- a/tools/enso4igv/src/main/java/org/enso/tools/enso4igv/enso/EnsoStructure.java +++ b/tools/enso4igv/src/main/java/org/enso/tools/enso4igv/enso/EnsoStructure.java @@ -32,9 +32,8 @@ static List collectStructure(Document dcmnt) { } var arr = new ArrayList(); try { - var parser = new EnsoParser(); var text = dcmnt.getText(0, dcmnt.getLength()); - var moduleIr = parser.compile(text); + var moduleIr = EnsoParser.compile(text); var it = moduleIr.bindings().iterator(); collectStructure(file, arr, it); return arr;