From ef5002f6abdda67250f709f49b3ef530512657e2 Mon Sep 17 00:00:00 2001 From: jakobeha Date: Thu, 1 Feb 2024 16:57:21 -0500 Subject: [PATCH 01/12] add Google style to intellij settings --- .idea/codeStyles/codeStyleConfig.xml | 2 +- .idea/settings.zip | Bin 21093 -> 21871 bytes pom.xml | 13 ++ src/main/java/org/prlprg/rds/RDSReader.java | 6 + src/test/java/org/prlprg/bc/CompilerTest.java | 53 +++++++- src/test/java/org/prlprg/util/Tests.java | 118 +++++++++++++++++- 6 files changed, 187 insertions(+), 5 deletions(-) diff --git a/.idea/codeStyles/codeStyleConfig.xml b/.idea/codeStyles/codeStyleConfig.xml index a55e7a179..b9d18bf59 100644 --- a/.idea/codeStyles/codeStyleConfig.xml +++ b/.idea/codeStyles/codeStyleConfig.xml @@ -1,5 +1,5 @@ - \ No newline at end of file diff --git a/.idea/settings.zip b/.idea/settings.zip index 6edb8b2bbebff9c0243284cf902e52e87aed5be6..ca768a5a93734a33481db5c852ee1e4031426a81 100644 GIT binary patch delta 2669 zcma)7c~p~E7XK2Kgg^*jkR`Gf0tt|?qns9tm5>lf0w|)Q5LqP3POvx}jZndXgDk$f z90jEnkWdR^9FaxAQ)*-vF3v@?^@mL_ zAHDV4?hi5UVeTR0FLzP2L+A2$eR(T+>%J-4o@Yr?t!bL|*xu3#N3o$2WO&a+PSi-Sx| zp<3OmES@W$#=F34JuLn>=J24*|Gm{^1S;J*y6ZtwHbW#Nu;-TvN@PJZwj#81G zmCp|qh34lg@|E%|r7SNyGe;Sz*pesD+Zn6O&XaFV+1CsfU)|%={+$j*)i6XN}>j10t!(Geg1y--d);$au;( zePNrpQrhl!(Xzn)`|!36v(s<5O9IbFa$MIfE4vWGp6zX62F=ECw6l(3kDABthjkAI zR(#NO{7d`UyN6?`b>%#{8K;mT+z zJkPH>TF<@PboZ$+w>P(sgr_qJ8&gEVRH_UXr4wZne-~~7Mr}YYO6qjLImAPQk08#V z*jOZIr=;@ol{+)wH#t2uGc~`~hpo~|MRcBeP(+t#g+ozvi5KU_pqL*=<`{DS!N-2x ztESUwdDF(_j2*9XrVIAZJkJ>SZ+(G$^B~y;n-?dLhWi;FoGvWu>eb80?MM?9C2^< zFWqWU4t=}{$q~}RFe8EFNFD+irr6YH!=3$*92XE!nj`@$t}}>`njq_&BUt1{GAN3$ zL(@tji+oxSgc>gay+RXYiXu<`A&-nx?X<_gw zpfubAf!;+q_V0kDu$_)!%moCL-crC8fiv>pLpY0c7O;e$AeF%~{zEsY;;e^ZOLbHV zXh2K12D?PWJ&Z*x@&jYwF0w;ttxL3X-nR zBrfQPWFs?05*E3Y3yw%!(ew^d_ce=r}zUq0deYjDT{=G z0x6NIy7u(H#lVviAh8xsgi>b!kOkXq{K#1R;?AS{!eRm!y!`K8_&-7kK7)y}2m%`< z_9#*PaM~XPRDSM2EhExj4@1Ac`(JeE+hCs5643fFi|HntiO-}yv_|Eyh|NM9K zYsvki63!~Rx%)FRnT+WxO@?y)0XOH_HH&v>WX%#140-pH)8fNB>NykZ3>r8d?kYYb z`Sw&Ea-}DTUzET*8r=^Q3{L5O0ay}UL;SOD7s!djU^w~~t+PCMD$YB7N2R=!usR$u zWpqlpo_7rTruh-55Mn2=cGiQ6n~V&pDBMuBNL4*L6;V0S+Wz-?x4e~xreeDfXwJ*u zZdCU4-h90M*(#k$wYKZQ$I52oK>x|U^a*px>3C+Z?W6J*!zt6D2MUDJ< z_8-1&a_Hc_xql7x58GGN&5s_;@;zwRUw#zO&yKa;={dA>TfxQg*VkMqHSD>U#Qauq zba{R9zo9!5HlE)$eD0E$EYDAWr~HlfvG4w<9LcQ~du!Lt?s@Y||05~0RV53a6r483 z7yBjJ<8lvY1m&dP4-Gq$vae&y#fjU+@)MgDo*8YOeWRf8xcBPh*kzZhGA=wAx>Mj= zw`^Q$6{WJ8Yl{638HQ<;FwFC(^B`~Me35%0Uy&_Yd_E_q^JTnzza_p}!UM;CF&7ZWnhCM?%jz3+)_7b2}oAPd5a@53kh{KiJqxPAC3mcpLHi zj8C8E9Mz`Hw3#z}Ki$B??Zp(J4Ci5K+%9L{VMz<5z&@iIJX0ytMZ(9LS!}F)in(hyAc^fzev#);53{+;! zC+$Ij z%1~%fS$>n@$>tqMdl}7@zk3$+&zb|jOargnEc+=BLGPid(O!O4a)^x%2BlR&`1c@* z)(gH#4<@17DzJ~eAjwL_MxofB6_6pSN%awm?ckt6u7bHD?JvXzwKr_BY6&99EUI(KIeor@~-fdNi4uZ4uZr;qbmi zLzL{EQ{r2T*sC*oY9b&~GJnR4Rb&ZlVI-)-180<$VaYQJ%60y*F^jHl67SFv4FOqN z!lt^_u)PbJHTcD)IIE91f_)!gALk;jE)AaL2&7M)E3l_bkdzy$jAEjgwF-tAH#x!o0rEaJ AH2?qr diff --git a/pom.xml b/pom.xml index 22fa19cf1..16d33a61c 100644 --- a/pom.xml +++ b/pom.xml @@ -59,6 +59,14 @@ + + + src/test/resources + + + src/test/R + + @@ -257,6 +265,11 @@ maven-surefire-plugin 3.2.2 + + + all + 8 + diff --git a/src/main/java/org/prlprg/rds/RDSReader.java b/src/main/java/org/prlprg/rds/RDSReader.java index 9ba05386d..4ecb5ed5b 100644 --- a/src/main/java/org/prlprg/rds/RDSReader.java +++ b/src/main/java/org/prlprg/rds/RDSReader.java @@ -6,6 +6,8 @@ import java.io.InputStream; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -34,6 +36,10 @@ public static SEXP readStream(InputStream input) throws IOException { } } + public static SEXP readFile(Path path) throws IOException { + return readStream(Files.newInputStream(path)); + } + private void readHeader() throws IOException { var type = in.readByte(); if (type != 'X') { diff --git a/src/test/java/org/prlprg/bc/CompilerTest.java b/src/test/java/org/prlprg/bc/CompilerTest.java index 0a85881ef..a2ba4a66f 100644 --- a/src/test/java/org/prlprg/bc/CompilerTest.java +++ b/src/test/java/org/prlprg/bc/CompilerTest.java @@ -1,16 +1,67 @@ package org.prlprg.bc; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.prlprg.util.Assertions.assertSnapshot; +import static org.prlprg.util.StructuralUtils.printStructurally; + +import java.io.IOException; +import java.nio.file.Path; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; import org.prlprg.compile.Compiler; import org.prlprg.rds.RDSReader; +import org.prlprg.sexp.BCodeSXP; import org.prlprg.sexp.CloSXP; +import org.prlprg.util.DirectorySource; +import org.prlprg.util.Files; +import org.prlprg.util.SubTest; import org.prlprg.util.Tests; public class CompilerTest implements Tests { @Test - public void testBasic() throws Exception { + public void testSomeOutput() throws IOException { var source = (CloSXP) RDSReader.readStream(getResourceAsStream("f1.rds")); var bc = Compiler.compileFun(source); System.out.println(bc); } + + @ParameterizedTest(name = "Commutative read/compile {0}") + @DirectorySource(glob = "*.R", relativize = true, exclude = "serialize-closures.R") + void testCommutativeReadAndCompile(Path sourceName) { + var sourcePath = getResourcePath(sourceName); + var compiledRoot = getSnapshotPath(sourceName); + + // Generate `compiledRoot` from `sourcePath` using GNU-R if necessary + if (Files.exists(compiledRoot) && Files.isOlder(compiledRoot, sourcePath)) { + Files.deleteRecursively(compiledRoot); + } + if (!Files.exists(compiledRoot)) { + // Generate `compiledRoot` from `sourcePath` using GNU-R + cmd( + "R", + "-s", + "-f", + getResourcePath("serialize-closures.R"), + "--args", + sourcePath, + compiledRoot); + } + + // Test each closure in the file + for (var astPath : Files.listDir(compiledRoot, "*.ast.rds", 1, false, false)) { + var name = astPath.getFileName().toString().split("\\.")[0]; + var bcPath = compiledRoot.resolve(name + ".bc.rds"); + var bcOutPath = compiledRoot.resolve(name + ".bc.out"); + SubTest.run( + name, + () -> { + var astClos = (CloSXP) RDSReader.readFile(astPath); + var bcClos = (CloSXP) RDSReader.readFile(bcPath); + var ourBc = printStructurally(Compiler.compileFun(astClos)); + var rBc = printStructurally(((BCodeSXP) bcClos.body()).bc()); + assertEquals(ourBc, rBc, "`compile(read(ast)) == read(R.compile(ast))`"); + assertSnapshot(bcOutPath, bcClos::toString, "`print(bc)`"); + }); + } + } } diff --git a/src/test/java/org/prlprg/util/Tests.java b/src/test/java/org/prlprg/util/Tests.java index b383b8875..c751fd44a 100644 --- a/src/test/java/org/prlprg/util/Tests.java +++ b/src/test/java/org/prlprg/util/Tests.java @@ -1,12 +1,124 @@ package org.prlprg.util; +import static org.prlprg.util.TestsPrivate.SNAPSHOT_RESOURCES_ROOT; + +import java.io.IOException; import java.io.InputStream; +import java.net.URL; +import java.nio.file.Path; +import java.util.Arrays; import java.util.Objects; public interface Tests { - default InputStream getResourceAsStream(String path) { + /** Reads the resource at {@code path}. {@code path} is relative to {@code anchor}'s package. */ + static InputStream getResourceAsStream(Class anchor, String path) { + return Objects.requireNonNull( + anchor.getResourceAsStream(path), + "Resource not found in " + anchor.getPackageName() + ": " + path); + } + + /** + * The absolute path of the snapshot resource at {@code path}. {@code path} is relative to {@code + * anchor}'s package. Be aware that it may not exist, although parent directories will be + * created if necessary. + */ + static Path getSnapshotPath(Class anchor, String path) { + if (path.startsWith("/")) { + throw new IllegalArgumentException("getSnapshotPath doesn't support absolute paths: " + path); + } + var snapshotPath = + SNAPSHOT_RESOURCES_ROOT.resolve(anchor.getPackageName().replace('.', '/')).resolve(path); + Files.createDirectories(snapshotPath.getParent()); + return snapshotPath; + } + + /** + * The absolute path of the snapshot resource at {@code path}. {@code path} is relative to {@code + * anchor}'s package. Be aware that it may not exist, although parent directories will be + * created if necessary. + */ + static Path getSnapshotPath(Class anchor, Path path) { + return getSnapshotPath(anchor, path.toString()); + } + + /** + * The absolute path of the resource at {@code path}. {@code path} is relative to {@code anchor}'s + * package. + */ + static Path getResourcePath(Class anchor, String path) { + return Files.pathFromFileUrl(getResource(anchor, path)); + } + + /** + * The URL of the resource at {@code path}. {@code path} is relative to {@code anchor}'s package. + */ + static URL getResource(Class anchor, String path) { return Objects.requireNonNull( - getClass().getResourceAsStream(path), - "Resource not found in " + getClass().getPackageName() + ": " + path); + anchor.getResource(path), "Resource not found in " + anchor.getPackageName() + ": " + path); + } + + /** Reads the resource at {@code path}. {@code path} is relative to this class's package. */ + default InputStream getResourceAsStream(String path) { + return getResourceAsStream(getClass(), path); } + + /** + * The absolute path of the resource at {@code path}. {@code path} is relative to this class's + * package. + */ + default Path getResourcePath(String path) { + return getResourcePath(getClass(), path); + } + + /** + * The absolute path of the resource at {@code path}. {@code path} is relative to this class's + * package. + */ + default Path getResourcePath(Path path) { + return getResourcePath(path.toString()); + } + + /** + * The absolute path of the snapshot resource at {@code path}. {@code path} is relative to {@code + * anchor}'s package. Be aware that it may not exist, although parent directories will be + * created if necessary. + */ + default Path getSnapshotPath(Path path) { + return getSnapshotPath(getClass(), path); + } + + /** Run a command. */ + default void cmd(Object... command) { + var commandStrs = Arrays.stream(command).map(Object::toString).toList(); + var commandStr = String.join(" ", commandStrs); + try { + var process = new ProcessBuilder(commandStrs).inheritIO().start(); + var exitCode = process.waitFor(); + if (exitCode != 0) { + throw new RuntimeException("Command failed with exit code " + exitCode + ": " + commandStr); + } + } catch (InterruptedException e) { + throw new RuntimeException("Command was interrupted: " + commandStr, e); + } catch (IOException e) { + throw new RuntimeException("Command failed with IO exception: " + commandStr, e); + } + } +} + +class TestsPrivate { + static final Path SNAPSHOT_RESOURCES_ROOT = getSnapshotPathsRoot(); + + private static Path getSnapshotPathsRoot() { + var basePath = Files.pathFromFileUrl(ClassLoader.getSystemResource(".")); + while (basePath != null && !basePath.endsWith("target")) { + basePath = basePath.getParent(); + } + if (basePath == null) { + System.err.println("Can't infer snapshot resources root, so we'll use a temporary directory"); + return Files.createTempDirectory(".snapshots"); + } + return basePath.getParent().resolve(".snapshots"); + } + + private TestsPrivate() {} } From 2ecfcd11bbf03580aa92615a6ffbb35b886408f9 Mon Sep 17 00:00:00 2001 From: jakobeha Date: Thu, 1 Feb 2024 16:59:18 -0500 Subject: [PATCH 02/12] add test harness, utils, and failing tests --- .../org/prlprg/bc/basics.R/arithmetic.ast.rds | Bin 0 -> 105 bytes .../org/prlprg/bc/basics.R/arithmetic.bc.rds | Bin 0 -> 314 bytes .../org/prlprg/bc/basics.R/calls.ast.rds | Bin 0 -> 206 bytes .../org/prlprg/bc/basics.R/calls.bc.rds | Bin 0 -> 971 bytes .../org/prlprg/bc/basics.R/conditions.ast.rds | Bin 0 -> 261 bytes .../org/prlprg/bc/basics.R/conditions.bc.rds | Bin 0 -> 814 bytes pom.xml | 2 + src/test/R/org/prlprg/bc/serialize-closures.R | 13 ++ src/test/java/org/prlprg/util/Assertions.java | 45 ++++++ .../java/org/prlprg/util/DirectorySource.java | 72 ++++++++++ src/test/java/org/prlprg/util/Files.java | 136 ++++++++++++++++++ .../java/org/prlprg/util/StructuralUtils.java | 38 +++++ src/test/java/org/prlprg/util/SubTest.java | 21 +++ .../org/prlprg/util/ThrowingRunnable.java | 7 + src/test/resources/org/prlprg/bc/basics.R | 3 + 15 files changed, 337 insertions(+) create mode 100644 .snapshots/org/prlprg/bc/basics.R/arithmetic.ast.rds create mode 100644 .snapshots/org/prlprg/bc/basics.R/arithmetic.bc.rds create mode 100644 .snapshots/org/prlprg/bc/basics.R/calls.ast.rds create mode 100644 .snapshots/org/prlprg/bc/basics.R/calls.bc.rds create mode 100644 .snapshots/org/prlprg/bc/basics.R/conditions.ast.rds create mode 100644 .snapshots/org/prlprg/bc/basics.R/conditions.bc.rds create mode 100644 src/test/R/org/prlprg/bc/serialize-closures.R create mode 100644 src/test/java/org/prlprg/util/Assertions.java create mode 100644 src/test/java/org/prlprg/util/DirectorySource.java create mode 100644 src/test/java/org/prlprg/util/Files.java create mode 100644 src/test/java/org/prlprg/util/StructuralUtils.java create mode 100644 src/test/java/org/prlprg/util/SubTest.java create mode 100644 src/test/java/org/prlprg/util/ThrowingRunnable.java create mode 100644 src/test/resources/org/prlprg/bc/basics.R diff --git a/.snapshots/org/prlprg/bc/basics.R/arithmetic.ast.rds b/.snapshots/org/prlprg/bc/basics.R/arithmetic.ast.rds new file mode 100644 index 0000000000000000000000000000000000000000..c591cb64cb3401f707b6fba1111889865d4391c4 GIT binary patch literal 105 zcma#xVqjokW?*4vVqj(kG8tGyL)>&N7=R)`&R-zS1f&@mSQt1#bRLlS4aEO|m<=QV Ylh6iofGQdPLudv*kfi+wAPE8g0Hx0e&;S4c literal 0 HcmV?d00001 diff --git a/.snapshots/org/prlprg/bc/basics.R/arithmetic.bc.rds b/.snapshots/org/prlprg/bc/basics.R/arithmetic.bc.rds new file mode 100644 index 0000000000000000000000000000000000000000..dde83e0bdb83172d062682655a9e0dbea12c1fcc GIT binary patch literal 314 zcmZ9HJr4mv5QgV&_XI(%;8&p6=_Gm*t#E}zj@af(KUpu1=RKcil4oaU=AGHuRol#* zc^8{YxLUGY%*GQVLvX{#_{hAqUfE~pkU-*B zA-dON>q$JlaDGi%hE`hD|1~V-s>ROF9xWjBj`*?K50lM)9RgMT;Jsm*6~|K+!tRiV KdA`jGYUKl4vKeOp literal 0 HcmV?d00001 diff --git a/.snapshots/org/prlprg/bc/basics.R/calls.ast.rds b/.snapshots/org/prlprg/bc/basics.R/calls.ast.rds new file mode 100644 index 0000000000000000000000000000000000000000..d2f680e9dcf0ccc118fdbc632cedf6f16256124c GIT binary patch literal 206 zcma#xVqjokW?*4vVqj(kG8tGyL)>&N7=R)`&R-zS1f&@mSQt1#bRv-WjV_pkO)wcE u$nXzHvjH*Oh%_JvWG0HBHbju|KZIri(IB1Z3fXc}^U_N)FjX;wbpQYy(GrOO literal 0 HcmV?d00001 diff --git a/.snapshots/org/prlprg/bc/basics.R/calls.bc.rds b/.snapshots/org/prlprg/bc/basics.R/calls.bc.rds new file mode 100644 index 0000000000000000000000000000000000000000..fa2456a87c1e57d8364a63536aa1f5f56c2b3ec7 GIT binary patch literal 971 zcmcJNIZwkt5QQh+wYh?n0)&J_K?4QQ^9zvZK}s8fIi$!SmLd|3|1)%m<9$vRt-*~8 zMtYfJpJ(3AX1s1@&b-Ua+$Y=5c^R|Yc=;!@+@v5shT@>w8ep~Ru zrZ8M$M}8pe35>uq5)b!4z~B_Z8;OjVp4gA98znU10Al>;5Ai4=!ux9H89By>X+z?! z$;GU!ZlCWGHepv6)m>VbL{H9c*vqQGZngyEoQi}sF!c%sfTO&)FQ~> z1A2hOT2Go!XOj?gIuhbSN9FwSsSM%%p$g~Kbvf4=D~V#`BfhAHZ3w#p1EcVL#fJh$ zJ&H$+@`B6Dx5N)?75f>NS)o@S{b-5HW*qwX^T{`l#FDYNR9}-VcyNVYb7P->z27&? C_fgRR literal 0 HcmV?d00001 diff --git a/.snapshots/org/prlprg/bc/basics.R/conditions.ast.rds b/.snapshots/org/prlprg/bc/basics.R/conditions.ast.rds new file mode 100644 index 0000000000000000000000000000000000000000..f2214dd9e19484cca53d472bcf383593cf201a8b GIT binary patch literal 261 zcma#xVqjokW?*4vVqj(kG8tGyL)>&N7=R)`&R-zS1f&@mSQt1#bRLlS4aEO|m<=QV zm0-$D1M)yhkwqEpAcBnlp^|(+nh}U$0AvhM6I>psnF*#A#UL9rg*XgCwOtoU6VxUK IY#RRo0K_#BTL1t6 literal 0 HcmV?d00001 diff --git a/.snapshots/org/prlprg/bc/basics.R/conditions.bc.rds b/.snapshots/org/prlprg/bc/basics.R/conditions.bc.rds new file mode 100644 index 0000000000000000000000000000000000000000..3eb88b68bc55be5b1375d4aa5ef4543044360ec6 GIT binary patch literal 814 zcmb7?KT88a5XEQjk3=z&LaRVNfRIwK5X*qQf~}Yn0^t(eCZ*pw>BQ*!tv9j>N#Ve| z*`0awZg!aGGczl#Dhn%X9j$(PoQ!Wx5naCI3;AG`o#I=4K83AtB~a2AM#8ypDFk64 zP$(g(|3WTtodeI$&c9Kt5~x9YC^+X@=My2<=iKY!^n`h{#q3Qtx*)R=FckKg;a))} z>KMLv?{+~@$PJNWHT(>I_Ol|_Bu{OY|7$zkb;+*Fe>a0T^E)U$J|M188cFt=qc3)@ zUY8nUGW*k6Lz3aq#4`;%jcH-Zv@{>O$jAY>AYT|=lQ}2%&Szl&3*&GeLvT1>erK!e d*ZE6~BC>_PJ%eVod~af$E!z0dPMei-+aDr+Hq`(C literal 0 HcmV?d00001 diff --git a/pom.xml b/pom.xml index 16d33a61c..10edc1f51 100644 --- a/pom.xml +++ b/pom.xml @@ -60,9 +60,11 @@ + src/test/resources + src/test/R diff --git a/src/test/R/org/prlprg/bc/serialize-closures.R b/src/test/R/org/prlprg/bc/serialize-closures.R new file mode 100644 index 000000000..474361b9f --- /dev/null +++ b/src/test/R/org/prlprg/bc/serialize-closures.R @@ -0,0 +1,13 @@ +.args <- commandArgs(trailingOnly = TRUE) +.source <- .args[1] +.target <- .args[2] + +source(.source, chdir = TRUE) +dir.create(.target) +for (.name in ls()) { + if (is.function(get(.name))) { + .func <- get(.name) + saveRDS(.func, compress = FALSE, file = file.path(.target, paste0(.name, ".ast.rds"))) + saveRDS(compiler::cmpfun(.func), compress = FALSE, file = file.path(.target, paste0(.name, ".bc.rds"))) + } +} \ No newline at end of file diff --git a/src/test/java/org/prlprg/util/Assertions.java b/src/test/java/org/prlprg/util/Assertions.java new file mode 100644 index 000000000..40c551ce4 --- /dev/null +++ b/src/test/java/org/prlprg/util/Assertions.java @@ -0,0 +1,45 @@ +package org.prlprg.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.nio.file.Path; +import java.util.function.Supplier; +import org.opentest4j.AssertionFailedError; + +public class Assertions { + /** + * First it asserts that producing the string twice results in the same string. + * + *

Then, if {@code snapshotPath} exists, it asserts that the produced string matches its + * content, and if not, writes the produced string to a file next to {@code snapshotPath}. If + * {@code snapshotPath} doesn't exist, it instead writes the produced string to it and logs that + * it was created. + * + * @param snapshotPath The absolute path to the snapshot file. + * @param snapshotName A human-readable name for the snapshot, used in the assertion messages. + */ + public static void assertSnapshot( + Path snapshotPath, Supplier actual, String snapshotName) { + if (!snapshotPath.isAbsolute()) { + throw new IllegalArgumentException( + "Snapshot path must be absolute: " + + snapshotPath + + "\nUse Tests.getSnapshot to get an absolute path from a relative one."); + } + var firstActual = actual.get(); + var secondActual = actual.get(); + assertEquals(firstActual, secondActual, "Non-determinism in " + snapshotName); + + if (!Files.exists(snapshotPath)) { + Files.writeString(snapshotPath, firstActual); + System.err.println("Created snapshot: " + snapshotPath); + } else { + var expected = Files.readString(snapshotPath); + if (!expected.equals(secondActual)) { + throw new AssertionFailedError("Regression in " + snapshotName, expected, secondActual); + } + } + } + + private Assertions() {} +} diff --git a/src/test/java/org/prlprg/util/DirectorySource.java b/src/test/java/org/prlprg/util/DirectorySource.java new file mode 100644 index 000000000..dbd41ec2d --- /dev/null +++ b/src/test/java/org/prlprg/util/DirectorySource.java @@ -0,0 +1,72 @@ +package org.prlprg.util; + +import com.google.common.collect.ImmutableSet; +import java.lang.annotation.*; +import java.util.stream.Stream; +import javax.annotation.Nullable; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.ArgumentsProvider; +import org.junit.jupiter.params.provider.ArgumentsSource; +import org.junit.jupiter.params.support.AnnotationConsumer; + +/** List all files in a directory and provide each one's path as an argument. */ +@Documented +@Target(ElementType.METHOD) +@Retention(RetentionPolicy.RUNTIME) +@ArgumentsSource(DirectoryArgumentsProvider.class) +public @interface DirectorySource { + /** Filter files by glob applied to the filename. Default is to not filter. */ + @Nullable String glob(); + + /** Whether to include directories. Default is to not. */ + boolean includeDirs() default false; + + /** Whether to relativize the paths to {@link #root}. Default is to not. */ + boolean relativize() default false; + + /** + * Specify a number > 1 to include files in subdirectories. Specify INT_MAX to recurse infinitely. + * Defaults to 1. + */ + int depth() default 1; + + /** + * Directory to list files from, defaults to corresponding resources directory of the test + * directory. Paths will be relative to this default unless they start with {@code /} + */ + String root() default ""; + + /** Paths to exclude. */ + String[] exclude() default {}; +} + +class DirectoryArgumentsProvider implements ArgumentsProvider, AnnotationConsumer { + private boolean accepted = false; + private @Nullable String glob; + private boolean includeDirs; + private boolean relativize; + private int depth; + private String root = ""; + private ImmutableSet exclude = ImmutableSet.of(); + + @Override + public void accept(DirectorySource directorySource) { + accepted = true; + glob = directorySource.glob(); + includeDirs = directorySource.includeDirs(); + relativize = directorySource.relativize(); + root = directorySource.root(); + depth = directorySource.depth(); + exclude = ImmutableSet.copyOf(directorySource.exclude()); + } + + @Override + public Stream provideArguments(ExtensionContext context) { + assert accepted; + var path = Tests.getResourcePath(context.getRequiredTestClass(), root); + return Files.listDir(path, glob, depth, includeDirs, relativize).stream() + .filter(p -> !exclude.contains(p.toString())) + .map(Arguments::of); + } +} diff --git a/src/test/java/org/prlprg/util/Files.java b/src/test/java/org/prlprg/util/Files.java new file mode 100644 index 000000000..109b6ad32 --- /dev/null +++ b/src/test/java/org/prlprg/util/Files.java @@ -0,0 +1,136 @@ +package org.prlprg.util; + +import java.io.IOException; +import java.net.URISyntaxException; +import java.net.URL; +import java.nio.file.FileSystems; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.attribute.FileTime; +import java.util.Collection; +import java.util.stream.Collectors; +import javax.annotation.Nullable; + +public class Files { + /** + * @param root Directory to list files from + * @param glob Filter files by glob applied to the filename. Pass {@code null} to not filter. + * @param depth Specify a number > 1 to include files in subdirectories. Specify INT_MAX to + * recurse infinitely. + * @param includeDirs Whether to include directories. + * @param relativize Whether to relativize the paths to {@code root}. + * @return The paths of each of the children of {@code root} filtered by the other arguments. It + * doesn't return {@code root} itself. + */ + public static Collection listDir( + Path root, @Nullable String glob, int depth, boolean includeDirs, boolean relativize) { + if (!isDirectory(root)) { + throw new IllegalArgumentException("Not a directory: " + root); + } + + var globMatcher = glob == null ? null : FileSystems.getDefault().getPathMatcher("glob:" + glob); + + try (var filesHandle = java.nio.file.Files.walk(root, depth)) { + var files = filesHandle.filter(file -> !file.equals(root)); + if (!includeDirs) { + files = files.filter(java.nio.file.Files::isRegularFile); + } + if (glob != null) { + files = files.filter(p -> globMatcher.matches(p.getFileName())); + } + if (relativize) { + files = files.map(root::relativize); + } + return files.collect(Collectors.toList()); + } catch (IOException e) { + throw new RuntimeException("Failed to list files in " + root, e); + } + } + + public static void deleteRecursively(Path path) { + try (var subpaths = java.nio.file.Files.walk(path)) { + subpaths + .sorted(java.util.Comparator.reverseOrder()) + .forEach( + subpath -> { + try { + java.nio.file.Files.delete(subpath); + } catch (IOException e) { + throw new RuntimeException( + "Failed to recursively delete " + + path + + ", specifically on " + + path.relativize(subpath), + e); + } + }); + } catch (IOException e) { + throw new RuntimeException("Failed to delete recursively " + path, e); + } + } + + public static boolean isOlder(Path lhs, Path rhs) { + return getLastModifiedTime(lhs).compareTo(getLastModifiedTime(rhs)) < 0; + } + + public static Path pathFromFileUrl(URL url) { + try { + return Paths.get(url.toURI()); + } catch (URISyntaxException e) { + throw new RuntimeException("Not a file URL: " + url, e); + } + } + + // region boilerplate wrappers which don't throw IOException + public static Path createTempDirectory(String prefix) { + try { + return java.nio.file.Files.createTempDirectory(prefix); + } catch (IOException e) { + throw new RuntimeException("Failed to create temp directory", e); + } + } + + public static void createDirectories(Path path) { + try { + java.nio.file.Files.createDirectories(path); + } catch (IOException e) { + throw new RuntimeException("Failed to create directories", e); + } + } + + public static FileTime getLastModifiedTime(Path path) { + try { + return java.nio.file.Files.getLastModifiedTime(path); + } catch (IOException e) { + throw new RuntimeException("Failed to get last modified time", e); + } + } + + public static void writeString(Path path, CharSequence out) { + try { + java.nio.file.Files.writeString(path, out); + } catch (IOException e) { + throw new RuntimeException("Failed to write string to " + path, e); + } + } + + public static String readString(Path path) { + try { + return java.nio.file.Files.readString(path); + } catch (IOException e) { + throw new RuntimeException("Failed to read string from " + path, e); + } + } + + public static boolean exists(Path path) { + return java.nio.file.Files.exists(path); + } + + public static boolean isDirectory(Path path) { + return java.nio.file.Files.isDirectory(path); + } + + // endregion + + private Files() {} +} diff --git a/src/test/java/org/prlprg/util/StructuralUtils.java b/src/test/java/org/prlprg/util/StructuralUtils.java new file mode 100644 index 000000000..20559c3ff --- /dev/null +++ b/src/test/java/org/prlprg/util/StructuralUtils.java @@ -0,0 +1,38 @@ +package org.prlprg.util; + +import java.util.regex.Pattern; + +public class StructuralUtils { + private static final Pattern HASH_PATTERN = Pattern.compile("@[0-9a-f]{1,16}"); + + /** + * Calls {@link Object#toString}, then replaces obvious references and hash-codes with + * deterministic values. This means that you can test that two objects are structurally equivalent + * by comparing their {@code printStructurally}. + * + *

It uses heuristics to find and references and hash-codes, so it can't be relied on for + * any data-structures we can't or aren't willing to change the {@link Object#toString} + * representation of to make them pass the tests. + */ + public static String printStructurally(Object object) { + var string = object.toString(); + var hashMatcher = HASH_PATTERN.matcher(string); + + // Get hashes in order of occurrence + var hashes = new java.util.LinkedHashSet(); + while (hashMatcher.find()) { + hashes.add(hashMatcher.group()); + } + + // Replace each hash with its index + var index = 0; + for (var hash : hashes) { + string = string.replace(hash, "@HASH" + index); + index++; + } + + return string; + } + + private StructuralUtils() {} +} diff --git a/src/test/java/org/prlprg/util/SubTest.java b/src/test/java/org/prlprg/util/SubTest.java new file mode 100644 index 000000000..18d8f7418 --- /dev/null +++ b/src/test/java/org/prlprg/util/SubTest.java @@ -0,0 +1,21 @@ +package org.prlprg.util; + +import org.opentest4j.AssertionFailedError; + +public class SubTest { + /** Runs the test and attaches {@code name} output and errors. */ + public static void run(String name, ThrowingRunnable test) { + System.out.println("- " + name); + try { + test.run(); + } catch (AssertionFailedError e) { + // Keep IntelliJ's `` + throw new AssertionFailedError( + "In " + name + ", " + e.getMessage(), e.getExpected(), e.getActual(), e.getCause()); + } catch (Throwable e) { + throw new RuntimeException("Failed " + name, e); + } + } + + private SubTest() {} +} diff --git a/src/test/java/org/prlprg/util/ThrowingRunnable.java b/src/test/java/org/prlprg/util/ThrowingRunnable.java new file mode 100644 index 000000000..dba34e018 --- /dev/null +++ b/src/test/java/org/prlprg/util/ThrowingRunnable.java @@ -0,0 +1,7 @@ +package org.prlprg.util; + +/** {@link Runnable} that may throw an error or exception. */ +@FunctionalInterface +public interface ThrowingRunnable { + void run() throws Throwable; +} diff --git a/src/test/resources/org/prlprg/bc/basics.R b/src/test/resources/org/prlprg/bc/basics.R new file mode 100644 index 000000000..1e04a8570 --- /dev/null +++ b/src/test/resources/org/prlprg/bc/basics.R @@ -0,0 +1,3 @@ +arithmetic <- function(n) n + 1 +calls <- function(a, b, c) f(a+b, length(b), c) +conditions <- function(n) if (n > 0) n else if (n < 0) -n else 0 \ No newline at end of file From e969a7176c8dba5fe06cabbd991dd8f39a1542c0 Mon Sep 17 00:00:00 2001 From: jakobeha Date: Thu, 1 Feb 2024 17:00:39 -0500 Subject: [PATCH 03/12] disable the failing test --- src/test/java/org/prlprg/bc/CompilerTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/java/org/prlprg/bc/CompilerTest.java b/src/test/java/org/prlprg/bc/CompilerTest.java index a2ba4a66f..a47a0cdf5 100644 --- a/src/test/java/org/prlprg/bc/CompilerTest.java +++ b/src/test/java/org/prlprg/bc/CompilerTest.java @@ -6,6 +6,7 @@ import java.io.IOException; import java.nio.file.Path; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.prlprg.compile.Compiler; @@ -25,6 +26,7 @@ public void testSomeOutput() throws IOException { System.out.println(bc); } + @Disabled @ParameterizedTest(name = "Commutative read/compile {0}") @DirectorySource(glob = "*.R", relativize = true, exclude = "serialize-closures.R") void testCommutativeReadAndCompile(Path sourceName) { From d0fa613e926ccff47ac624cc1cd7f31b6505943b Mon Sep 17 00:00:00 2001 From: jakobeha Date: Fri, 2 Feb 2024 07:06:17 -0500 Subject: [PATCH 04/12] fix pre-commit check for ambiguous after reformat (was inverted) --- .githooks/pre-commit.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.githooks/pre-commit.sh b/.githooks/pre-commit.sh index 4dffac2f8..4aec57173 100755 --- a/.githooks/pre-commit.sh +++ b/.githooks/pre-commit.sh @@ -2,7 +2,7 @@ everything_staged=$(git diff --name-only) -if [ -z "$everything_staged" ]; then +if [ -n "$everything_staged" ]; then # Don't commit because, after we reformat, the formatting will be unstaged, but it's too hard to determine (and maybe # ambiguous) what are the formatting differences and what was originally unstaged. mvn spotless:check || { From 9c525d690e43538a7131824e146e9f68d2d21555 Mon Sep 17 00:00:00 2001 From: jakobeha Date: Fri, 2 Feb 2024 07:07:41 -0500 Subject: [PATCH 05/12] fix assertSnapshot so we actually write the failing snapshot and improve so we automatically delete it --- src/test/java/org/prlprg/util/Assertions.java | 17 ++++++++++---- src/test/java/org/prlprg/util/Files.java | 16 ++++++++++++- src/test/java/org/prlprg/util/Paths.java | 23 +++++++++++++++++++ 3 files changed, 51 insertions(+), 5 deletions(-) create mode 100644 src/test/java/org/prlprg/util/Paths.java diff --git a/src/test/java/org/prlprg/util/Assertions.java b/src/test/java/org/prlprg/util/Assertions.java index 40c551ce4..ab15a03b1 100644 --- a/src/test/java/org/prlprg/util/Assertions.java +++ b/src/test/java/org/prlprg/util/Assertions.java @@ -11,9 +11,13 @@ public class Assertions { * First it asserts that producing the string twice results in the same string. * *

Then, if {@code snapshotPath} exists, it asserts that the produced string matches its - * content, and if not, writes the produced string to a file next to {@code snapshotPath}. If - * {@code snapshotPath} doesn't exist, it instead writes the produced string to it and logs that - * it was created. + * content. If {@code snapshotPath} doesn't exist, it instead writes the produced string to it and + * logs that it was created. + * + *

Additionally, test failures are written to a file next to {@code snapshotPath} which gets + * automatically deleted after the test passes; this file can be diffed with the snapshot to see + * the regression, and if the snapshot was supposed to change it can be moved to {@code + * snapshotPath} to update it. * * @param snapshotPath The absolute path to the snapshot file. * @param snapshotName A human-readable name for the snapshot, used in the assertion messages. @@ -30,12 +34,17 @@ public static void assertSnapshot( var secondActual = actual.get(); assertEquals(firstActual, secondActual, "Non-determinism in " + snapshotName); + var failingSnapshotPath = + Paths.withExtension(snapshotPath, ".fail" + Paths.getExtension(snapshotPath)); + Files.deleteIfExists(failingSnapshotPath); + if (!Files.exists(snapshotPath)) { - Files.writeString(snapshotPath, firstActual); + Files.writeString(snapshotPath, secondActual); System.err.println("Created snapshot: " + snapshotPath); } else { var expected = Files.readString(snapshotPath); if (!expected.equals(secondActual)) { + Files.writeString(failingSnapshotPath, secondActual); throw new AssertionFailedError("Regression in " + snapshotName, expected, secondActual); } } diff --git a/src/test/java/org/prlprg/util/Files.java b/src/test/java/org/prlprg/util/Files.java index 109b6ad32..3a79994ef 100644 --- a/src/test/java/org/prlprg/util/Files.java +++ b/src/test/java/org/prlprg/util/Files.java @@ -1,5 +1,6 @@ package org.prlprg.util; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.io.IOException; import java.net.URISyntaxException; import java.net.URL; @@ -33,7 +34,7 @@ public static Collection listDir( try (var filesHandle = java.nio.file.Files.walk(root, depth)) { var files = filesHandle.filter(file -> !file.equals(root)); if (!includeDirs) { - files = files.filter(java.nio.file.Files::isRegularFile); + files = files.filter(Files::isRegularFile); } if (glob != null) { files = files.filter(p -> globMatcher.matches(p.getFileName())); @@ -122,6 +123,15 @@ public static String readString(Path path) { } } + @CanIgnoreReturnValue + public static boolean deleteIfExists(Path path) { + try { + return java.nio.file.Files.deleteIfExists(path); + } catch (IOException e) { + throw new RuntimeException("Failed to delete " + path, e); + } + } + public static boolean exists(Path path) { return java.nio.file.Files.exists(path); } @@ -130,6 +140,10 @@ public static boolean isDirectory(Path path) { return java.nio.file.Files.isDirectory(path); } + public static boolean isRegularFile(Path path) { + return java.nio.file.Files.isRegularFile(path); + } + // endregion private Files() {} diff --git a/src/test/java/org/prlprg/util/Paths.java b/src/test/java/org/prlprg/util/Paths.java new file mode 100644 index 000000000..3af409a0b --- /dev/null +++ b/src/test/java/org/prlprg/util/Paths.java @@ -0,0 +1,23 @@ +package org.prlprg.util; + +import java.nio.file.Path; + +public class Paths { + public static String getFileStem(Path path) { + var fileName = path.getFileName().toString(); + var lastDot = fileName.lastIndexOf('.'); + return lastDot == -1 ? fileName : fileName.substring(0, lastDot); + } + + public static String getExtension(Path path) { + var fileName = path.getFileName().toString(); + var lastDot = fileName.lastIndexOf('.'); + return lastDot == -1 ? "" : fileName.substring(lastDot); + } + + public static Path withExtension(Path path, String extension) { + return path.resolveSibling(getFileStem(path) + extension); + } + + private Paths() {} +} From 6669e388930eaa4894ef9c39615e07b2980ea669 Mon Sep 17 00:00:00 2001 From: jakobeha Date: Fri, 2 Feb 2024 09:14:22 -0500 Subject: [PATCH 06/12] Add failing snapshots to .gitignore --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 8a76becaa..d34a28056 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,6 @@ +.snapshots/**/*.fail* + +### Build ### target/ !.mvn/wrapper/maven-wrapper.jar !**/src/main/**/target/ From 3e1445d60b4f85450bd738131d7e18abaa5c61a9 Mon Sep 17 00:00:00 2001 From: jakobeha Date: Sat, 3 Feb 2024 20:19:42 -0500 Subject: [PATCH 07/12] Add short package-into descriptions --- src/main/java/org/prlprg/bc/package-info.java | 1 + src/main/java/org/prlprg/compile/package-info.java | 1 + src/main/java/org/prlprg/primitive/Names.java | 1 + src/main/java/org/prlprg/primitive/package-info.java | 1 + src/main/java/org/prlprg/rds/package-info.java | 1 + src/main/java/org/prlprg/sexp/package-info.java | 1 + src/main/java/org/prlprg/util/PackagePrivate.java | 7 ------- src/main/java/org/prlprg/util/package-info.java | 1 + 8 files changed, 7 insertions(+), 7 deletions(-) delete mode 100644 src/main/java/org/prlprg/util/PackagePrivate.java diff --git a/src/main/java/org/prlprg/bc/package-info.java b/src/main/java/org/prlprg/bc/package-info.java index 805179475..d42a28101 100644 --- a/src/main/java/org/prlprg/bc/package-info.java +++ b/src/main/java/org/prlprg/bc/package-info.java @@ -1,3 +1,4 @@ +/** GNU-R bytecode representation. */ @ParametersAreNonnullByDefault @FieldsAreNonNullByDefault @ReturnTypesAreNonNullByDefault diff --git a/src/main/java/org/prlprg/compile/package-info.java b/src/main/java/org/prlprg/compile/package-info.java index 1358783b4..a47d16b82 100644 --- a/src/main/java/org/prlprg/compile/package-info.java +++ b/src/main/java/org/prlprg/compile/package-info.java @@ -1,3 +1,4 @@ +/** Compile GNU-R ASTs (S-expressions) into bytecode. */ @ParametersAreNonnullByDefault @FieldsAreNonNullByDefault @ReturnTypesAreNonNullByDefault diff --git a/src/main/java/org/prlprg/primitive/Names.java b/src/main/java/org/prlprg/primitive/Names.java index ed155b746..216bdaa56 100644 --- a/src/main/java/org/prlprg/primitive/Names.java +++ b/src/main/java/org/prlprg/primitive/Names.java @@ -2,6 +2,7 @@ import com.google.common.collect.ImmutableList; +/** Special symbols */ public final class Names { public static final ImmutableList BINOPS = ImmutableList.of( diff --git a/src/main/java/org/prlprg/primitive/package-info.java b/src/main/java/org/prlprg/primitive/package-info.java index 8fd3c5c4b..1fcb4b9c9 100644 --- a/src/main/java/org/prlprg/primitive/package-info.java +++ b/src/main/java/org/prlprg/primitive/package-info.java @@ -1,3 +1,4 @@ +/** GNU-R simple struct datatypes and values which aren't s-exoressions or contexts. */ @ParametersAreNonnullByDefault @FieldsAreNonNullByDefault @ReturnTypesAreNonNullByDefault diff --git a/src/main/java/org/prlprg/rds/package-info.java b/src/main/java/org/prlprg/rds/package-info.java index 71645c59d..66870f111 100644 --- a/src/main/java/org/prlprg/rds/package-info.java +++ b/src/main/java/org/prlprg/rds/package-info.java @@ -1,3 +1,4 @@ +/** Deserialize (and later serialize) S-expressions. */ @ParametersAreNonnullByDefault @FieldsAreNonNullByDefault @ReturnTypesAreNonNullByDefault diff --git a/src/main/java/org/prlprg/sexp/package-info.java b/src/main/java/org/prlprg/sexp/package-info.java index 81d89a94c..f9b817754 100644 --- a/src/main/java/org/prlprg/sexp/package-info.java +++ b/src/main/java/org/prlprg/sexp/package-info.java @@ -1,3 +1,4 @@ +/** S-expressions (GNU-R object type): the different types and associated data. */ @ParametersAreNonnullByDefault @FieldsAreNonNullByDefault @ReturnTypesAreNonNullByDefault diff --git a/src/main/java/org/prlprg/util/PackagePrivate.java b/src/main/java/org/prlprg/util/PackagePrivate.java deleted file mode 100644 index 747c21222..000000000 --- a/src/main/java/org/prlprg/util/PackagePrivate.java +++ /dev/null @@ -1,7 +0,0 @@ -package org.prlprg.util; - -/** - * Explicitly denotes that a member is package-private. Otherwise, pmd will warn in case you forgot - * to add a visibility. - */ -public @interface PackagePrivate {} diff --git a/src/main/java/org/prlprg/util/package-info.java b/src/main/java/org/prlprg/util/package-info.java index c222aa8cf..f062e0173 100644 --- a/src/main/java/org/prlprg/util/package-info.java +++ b/src/main/java/org/prlprg/util/package-info.java @@ -1,3 +1,4 @@ +/** Misc stuff which could go in any package. */ @ParametersAreNonnullByDefault @FieldsAreNonNullByDefault @ReturnTypesAreNonNullByDefault From 17575a602abef01cde0dcdf140f7836ca44c24c7 Mon Sep 17 00:00:00 2001 From: jakobeha Date: Sat, 3 Feb 2024 20:32:37 -0500 Subject: [PATCH 08/12] fix labels so they have the correct offsets GNU-R bytecode contains both instructions and metadata, ours only contains instructions, so the indices of our instructions are different. Since labels are offsets in the bytecode, this means they need to be different as well. Of course, if the labels are only used by GNU-R and we don't modify and re-encode the bytecode this is unnecessary. But, I'm sure we'll use the labels and their offsets while compiling, even if just to find the position to generate native code --- src/main/java/org/prlprg/bc/BcCode.java | 47 ++++- src/main/java/org/prlprg/bc/BcInstr.java | 210 +++++++++++++++++--- src/main/java/org/prlprg/bc/BcLabel.java | 114 ++++++++++- src/main/java/org/prlprg/rds/RDSReader.java | 7 +- src/main/java/org/prlprg/sexp/IntSXP.java | 30 +-- src/main/java/org/prlprg/sexp/SEXP.java | 5 + 6 files changed, 355 insertions(+), 58 deletions(-) diff --git a/src/main/java/org/prlprg/bc/BcCode.java b/src/main/java/org/prlprg/bc/BcCode.java index eae3f307d..ddefec229 100644 --- a/src/main/java/org/prlprg/bc/BcCode.java +++ b/src/main/java/org/prlprg/bc/BcCode.java @@ -32,13 +32,54 @@ protected List delegate() { */ static BcCode fromRaw(ImmutableIntArray bytecodes, ConstPool.MakeIdx makePoolIdx) throws BcFromRawException { + if (bytecodes.isEmpty()) { + throw new BcFromRawException("Bytecode is empty, needs at least version number"); + } + if (bytecodes.get(0) != Bc.R_BC_VERSION) { + throw new BcFromRawException("Unsupported bytecode version: " + bytecodes.get(0)); + } + + var labelMap = labelFactoryFromRaw(bytecodes); + var builder = new Builder(); - int i = 0; + int i = 1; + int sanityCheckJ = 0; while (i < bytecodes.length()) { try { - var instrAndI = BcInstrs.fromRaw(bytecodes, i, makePoolIdx); - builder.add(instrAndI.a()); + var instrAndI = BcInstrs.fromRaw(bytecodes, i, labelMap, makePoolIdx); + var instr = instrAndI.a(); i = instrAndI.b(); + + builder.add(instr); + sanityCheckJ++; + + try { + var sanityCheckJFromI = labelMap.make(i).target; + if (sanityCheckJFromI != sanityCheckJ) { + throw new AssertionError( + "expected target offset " + sanityCheckJ + ", got " + sanityCheckJFromI); + } + } catch (IllegalArgumentException | AssertionError e) { + throw new AssertionError( + "BcInstrs.fromRaw and BcInstrs.sizeFromRaw are out of sync, at instruction " + instr, + e); + } + } catch (BcFromRawException e) { + throw new BcFromRawException( + "malformed bytecode at " + i + "\nBytecode up to this point: " + builder.build(), e); + } + } + return builder.build(); + } + + static BcLabel.Factory labelFactoryFromRaw(ImmutableIntArray bytecodes) { + var builder = new BcLabel.Factory.Builder(); + int i = 1; + while (i < bytecodes.length()) { + try { + var size = BcInstrs.sizeFromRaw(bytecodes, i); + builder.step(size, 1); + i += size; } catch (BcFromRawException e) { throw new BcFromRawException( "malformed bytecode at " + i + "\nBytecode up to this point: " + builder.build(), e); diff --git a/src/main/java/org/prlprg/bc/BcInstr.java b/src/main/java/org/prlprg/bc/BcInstr.java index 8e3e0e7c2..8071d98fa 100644 --- a/src/main/java/org/prlprg/bc/BcInstr.java +++ b/src/main/java/org/prlprg/bc/BcInstr.java @@ -944,21 +944,21 @@ class BcInstrs { /** * Create from the raw GNU-R representation. * - * @param bytecodes The full list of instruction bytecodes including ones before and after this - * one - * @param i The index in the list where this instruction starts - * @param makePoolIdx A function to create pool indices from raw integers - * @return The instruction and the index in the list where the next instruction starts + * @param bytecodes The full list of GNU-R bytecodes including ones before and after this one. + * @param i The index in the list where this instruction starts. + * @param Label So we can create labels from GNU-R labels. + * @param makePoolIdx A function to create pool indices from raw integers. + * @return The instruction and the index in the list where the next instruction starts. * @apiNote This has to be in a separate class because it's package-private but interface methods * are public. */ static Pair fromRaw( - ImmutableIntArray bytecodes, int i, ConstPool.MakeIdx makePoolIdx) { + ImmutableIntArray bytecodes, int i, BcLabel.Factory Label, ConstPool.MakeIdx makePoolIdx) { BcOp op; try { op = BcOp.valueOf(bytecodes.get(i++)); } catch (IllegalArgumentException e) { - throw new BcFromRawException("invalid opcode (instruction) " + bytecodes.get(i - 1)); + throw new BcFromRawException("invalid opcode (instruction) at " + bytecodes.get(i - 1)); } try { @@ -967,16 +967,15 @@ static Pair fromRaw( case BCMISMATCH -> throw new BcFromRawException("invalid opcode " + BcOp.BCMISMATCH.value()); case RETURN -> new BcInstr.Return(); - case GOTO -> new BcInstr.Goto(new BcLabel(bytecodes.get(i++))); + case GOTO -> new BcInstr.Goto(Label.make(bytecodes.get(i++))); case BRIFNOT -> new BcInstr.BrIfNot( - makePoolIdx.lang(bytecodes.get(i++)), new BcLabel(bytecodes.get(i++))); + makePoolIdx.lang(bytecodes.get(i++)), Label.make(bytecodes.get(i++))); case POP -> new BcInstr.Pop(); case DUP -> new BcInstr.Dup(); case PRINTVALUE -> new BcInstr.PrintValue(); case STARTLOOPCNTXT -> - new BcInstr.StartLoopCntxt( - bytecodes.get(i++) != 0, new BcLabel(bytecodes.get(i++))); + new BcInstr.StartLoopCntxt(bytecodes.get(i++) != 0, Label.make(bytecodes.get(i++))); case ENDLOOPCNTXT -> new BcInstr.EndLoopCntxt(bytecodes.get(i++) != 0); case DOLOOPNEXT -> new BcInstr.DoLoopNext(); case DOLOOPBREAK -> new BcInstr.DoLoopBreak(); @@ -984,8 +983,8 @@ static Pair fromRaw( new BcInstr.StartFor( makePoolIdx.lang(bytecodes.get(i++)), makePoolIdx.sym(bytecodes.get(i++)), - new BcLabel(bytecodes.get(i++))); - case STEPFOR -> new BcInstr.StepFor(new BcLabel(bytecodes.get(i++))); + Label.make(bytecodes.get(i++))); + case STEPFOR -> new BcInstr.StepFor(Label.make(bytecodes.get(i++))); case ENDFOR -> new BcInstr.EndFor(); case SETLOOPVAL -> new BcInstr.SetLoopVal(); case INVISIBLE -> new BcInstr.Invisible(); @@ -1039,23 +1038,23 @@ static Pair fromRaw( case ENDASSIGN -> new BcInstr.EndAssign(makePoolIdx.sym(bytecodes.get(i++))); case STARTSUBSET -> new BcInstr.StartSubset( - makePoolIdx.lang(bytecodes.get(i++)), new BcLabel(bytecodes.get(i++))); + makePoolIdx.lang(bytecodes.get(i++)), Label.make(bytecodes.get(i++))); case DFLTSUBSET -> new BcInstr.DfltSubset(); case STARTSUBASSIGN -> new BcInstr.StartSubassign( - makePoolIdx.lang(bytecodes.get(i++)), new BcLabel(bytecodes.get(i++))); + makePoolIdx.lang(bytecodes.get(i++)), Label.make(bytecodes.get(i++))); case DFLTSUBASSIGN -> new BcInstr.DfltSubassign(); case STARTC -> new BcInstr.StartC( - makePoolIdx.lang(bytecodes.get(i++)), new BcLabel(bytecodes.get(i++))); + makePoolIdx.lang(bytecodes.get(i++)), Label.make(bytecodes.get(i++))); case DFLTC -> new BcInstr.DfltC(); case STARTSUBSET2 -> new BcInstr.StartSubset2( - makePoolIdx.lang(bytecodes.get(i++)), new BcLabel(bytecodes.get(i++))); + makePoolIdx.lang(bytecodes.get(i++)), Label.make(bytecodes.get(i++))); case DFLTSUBSET2 -> new BcInstr.DfltSubset2(); case STARTSUBASSIGN2 -> new BcInstr.StartSubassign2( - makePoolIdx.lang(bytecodes.get(i++)), new BcLabel(bytecodes.get(i++))); + makePoolIdx.lang(bytecodes.get(i++)), Label.make(bytecodes.get(i++))); case DFLTSUBASSIGN2 -> new BcInstr.DfltSubassign2(); case DOLLAR -> new BcInstr.Dollar( @@ -1080,11 +1079,11 @@ static Pair fromRaw( new BcInstr.MatSubassign(makePoolIdx.langOrNegative(bytecodes.get(i++))); case AND1ST -> new BcInstr.And1st( - makePoolIdx.lang(bytecodes.get(i++)), new BcLabel(bytecodes.get(i++))); + makePoolIdx.lang(bytecodes.get(i++)), Label.make(bytecodes.get(i++))); case AND2ND -> new BcInstr.And2nd(makePoolIdx.lang(bytecodes.get(i++))); case OR1ST -> new BcInstr.Or1st( - makePoolIdx.lang(bytecodes.get(i++)), new BcLabel(bytecodes.get(i++))); + makePoolIdx.lang(bytecodes.get(i++)), Label.make(bytecodes.get(i++))); case OR2ND -> new BcInstr.Or2nd(makePoolIdx.lang(bytecodes.get(i++))); case GETVAR_MISSOK -> new BcInstr.GetVarMissOk(makePoolIdx.sym(bytecodes.get(i++))); case DDVAL_MISSOK -> new BcInstr.DdValMissOk(makePoolIdx.sym(bytecodes.get(i++))); @@ -1107,10 +1106,10 @@ static Pair fromRaw( case RETURNJMP -> new BcInstr.ReturnJmp(); case STARTSUBSET_N -> new BcInstr.StartSubsetN( - makePoolIdx.lang(bytecodes.get(i++)), new BcLabel(bytecodes.get(i++))); + makePoolIdx.lang(bytecodes.get(i++)), Label.make(bytecodes.get(i++))); case STARTSUBASSIGN_N -> new BcInstr.StartSubassignN( - makePoolIdx.lang(bytecodes.get(i++)), new BcLabel(bytecodes.get(i++))); + makePoolIdx.lang(bytecodes.get(i++)), Label.make(bytecodes.get(i++))); case VECSUBSET2 -> new BcInstr.VecSubset2(makePoolIdx.langOrNegative(bytecodes.get(i++))); case MATSUBSET2 -> @@ -1121,10 +1120,10 @@ static Pair fromRaw( new BcInstr.MatSubassign2(makePoolIdx.langOrNegative(bytecodes.get(i++))); case STARTSUBSET2_N -> new BcInstr.StartSubset2N( - makePoolIdx.lang(bytecodes.get(i++)), new BcLabel(bytecodes.get(i++))); + makePoolIdx.lang(bytecodes.get(i++)), Label.make(bytecodes.get(i++))); case STARTSUBASSIGN2_N -> new BcInstr.StartSubassign2N( - makePoolIdx.lang(bytecodes.get(i++)), new BcLabel(bytecodes.get(i++))); + makePoolIdx.lang(bytecodes.get(i++)), Label.make(bytecodes.get(i++))); case SUBSET_N -> new BcInstr.SubsetN( makePoolIdx.langOrNegative(bytecodes.get(i++)), bytecodes.get(i++)); @@ -1148,7 +1147,7 @@ static Pair fromRaw( case SEQLEN -> new BcInstr.SeqLen(makePoolIdx.lang(bytecodes.get(i++))); case BASEGUARD -> new BcInstr.BaseGuard( - makePoolIdx.lang(bytecodes.get(i++)), new BcLabel(bytecodes.get(i++))); + makePoolIdx.lang(bytecodes.get(i++)), Label.make(bytecodes.get(i++))); case INCLNK -> new BcInstr.IncLnk(); case DECLNK -> new BcInstr.DecLnk(); case DECLNK_N -> new BcInstr.DeclnkN(bytecodes.get(i++)); @@ -1156,6 +1155,167 @@ static Pair fromRaw( case DECLNKSTK -> new BcInstr.DecLnkStk(); }; return new Pair<>(instr, i); + } catch (IllegalArgumentException e) { + throw new BcFromRawException("invalid opcode " + op + " (arguments)", e); + } catch (ArrayIndexOutOfBoundsException e) { + throw new BcFromRawException( + "invalid opcode " + op + " (arguments, unexpected end of bytecode stream)"); + } + } + + /** + * Get the GNU-R size of the instruction at the position without creating it. + * + * @param bytecodes The full list of GNU-R bytecodes including ones before and after this one. + * @param i The index in the list where this instruction starts. + * @return The size of the instruction (we don't return next position because it can be computed + * from this). + * @apiNote This has to be in a separate class because it's package-private but interface methods + * are public. + */ + @SuppressWarnings({"DuplicateBranchesInSwitch", "DuplicatedCode"}) + static int sizeFromRaw(ImmutableIntArray bytecodes, int i) { + BcOp op; + try { + op = BcOp.valueOf(bytecodes.get(i++)); + } catch (IllegalArgumentException e) { + throw new BcFromRawException("invalid opcode (instruction) " + bytecodes.get(i - 1)); + } + + try { + return 1 + + switch (op) { + case BCMISMATCH -> + throw new BcFromRawException("invalid opcode " + BcOp.BCMISMATCH.value()); + case RETURN -> 0; + case GOTO -> 1; + case BRIFNOT -> 2; + case POP -> 0; + case DUP -> 0; + case PRINTVALUE -> 0; + case STARTLOOPCNTXT -> 2; + case ENDLOOPCNTXT -> 1; + case DOLOOPNEXT -> 0; + case DOLOOPBREAK -> 0; + case STARTFOR -> 3; + case STEPFOR -> 1; + case ENDFOR -> 0; + case SETLOOPVAL -> 0; + case INVISIBLE -> 0; + case LDCONST -> 1; + case LDNULL -> 0; + case LDTRUE -> 0; + case LDFALSE -> 0; + case GETVAR -> 1; + case DDVAL -> 1; + case SETVAR -> 1; + case GETFUN -> 1; + case GETGLOBFUN -> 1; + case GETSYMFUN -> 1; + case GETBUILTIN -> 1; + case GETINTLBUILTIN -> 1; + case CHECKFUN -> 0; + case MAKEPROM -> 1; + case DOMISSING -> 0; + case SETTAG -> 1; + case DODOTS -> 0; + case PUSHARG -> 0; + case PUSHCONSTARG -> 1; + case PUSHNULLARG -> 0; + case PUSHTRUEARG -> 0; + case PUSHFALSEARG -> 0; + case CALL -> 1; + case CALLBUILTIN -> 1; + case CALLSPECIAL -> 1; + case MAKECLOSURE -> 1; + case UMINUS -> 1; + case UPLUS -> 1; + case ADD -> 1; + case SUB -> 1; + case MUL -> 1; + case DIV -> 1; + case EXPT -> 1; + case SQRT -> 1; + case EXP -> 1; + case EQ -> 1; + case NE -> 1; + case LT -> 1; + case LE -> 1; + case GE -> 1; + case GT -> 1; + case AND -> 1; + case OR -> 1; + case NOT -> 1; + case DOTSERR -> 0; + case STARTASSIGN -> 1; + case ENDASSIGN -> 1; + case STARTSUBSET -> 2; + case DFLTSUBSET -> 0; + case STARTSUBASSIGN -> 2; + case DFLTSUBASSIGN -> 0; + case STARTC -> 2; + case DFLTC -> 0; + case STARTSUBSET2 -> 2; + case DFLTSUBSET2 -> 0; + case STARTSUBASSIGN2 -> 2; + case DFLTSUBASSIGN2 -> 0; + case DOLLAR -> 2; + case DOLLARGETS -> 2; + case ISNULL -> 0; + case ISLOGICAL -> 0; + case ISINTEGER -> 0; + case ISDOUBLE -> 0; + case ISCOMPLEX -> 0; + case ISCHARACTER -> 0; + case ISSYMBOL -> 0; + case ISOBJECT -> 0; + case ISNUMERIC -> 0; + case VECSUBSET -> 1; + case MATSUBSET -> 1; + case VECSUBASSIGN -> 1; + case MATSUBASSIGN -> 1; + case AND1ST -> 2; + case AND2ND -> 1; + case OR1ST -> 2; + case OR2ND -> 1; + case GETVAR_MISSOK -> 1; + case DDVAL_MISSOK -> 1; + case VISIBLE -> 0; + case SETVAR2 -> 1; + case STARTASSIGN2 -> 1; + case ENDASSIGN2 -> 1; + case SETTER_CALL -> 2; + case GETTER_CALL -> 1; + case SWAP -> 0; + case DUP2ND -> 0; + case SWITCH -> 4; + case RETURNJMP -> 0; + case STARTSUBSET_N -> 2; + case STARTSUBASSIGN_N -> 2; + case VECSUBSET2 -> 1; + case MATSUBSET2 -> 1; + case VECSUBASSIGN2 -> 1; + case MATSUBASSIGN2 -> 1; + case STARTSUBSET2_N -> 2; + case STARTSUBASSIGN2_N -> 2; + case SUBSET_N -> 2; + case SUBSET2_N -> 2; + case SUBASSIGN_N -> 2; + case SUBASSIGN2_N -> 2; + case LOG -> 1; + case LOGBASE -> 1; + case MATH1 -> 2; + case DOTCALL -> 2; + case COLON -> 1; + case SEQALONG -> 1; + case SEQLEN -> 1; + case BASEGUARD -> 2; + case INCLNK -> 0; + case DECLNK -> 0; + case DECLNK_N -> 1; + case INCLNKSTK -> 0; + case DECLNKSTK -> 0; + }; } catch (IllegalArgumentException e) { throw new BcFromRawException("invalid opcode (arguments) " + op, e); } catch (ArrayIndexOutOfBoundsException e) { diff --git a/src/main/java/org/prlprg/bc/BcLabel.java b/src/main/java/org/prlprg/bc/BcLabel.java index 5dbdb7b6a..a3aca42c5 100644 --- a/src/main/java/org/prlprg/bc/BcLabel.java +++ b/src/main/java/org/prlprg/bc/BcLabel.java @@ -1,3 +1,115 @@ package org.prlprg.bc; -public record BcLabel(int id) {} +import com.google.common.base.Objects; +import com.google.common.primitives.ImmutableIntArray; +import com.google.errorprone.annotations.CanIgnoreReturnValue; + +/** A branch instruction destination. */ +public final class BcLabel { + /** Index of the instruction the branch instruction jumps to. */ + public final int target; + + private BcLabel(int target) { + this.target = target; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof BcLabel bcLabel)) return false; + return target == bcLabel.target; + } + + @Override + public int hashCode() { + return Objects.hashCode(target); + } + + @Override + public String toString() { + return "BcLabel(" + target + ')'; + } + + /** + * Create labels from GNU-R labels. + * + * @implNote This contains a map of positions in GNU-R bytecode to positions in our bytecode. We + * need this because every index in our bytecode maps to an instruction, while indexes in + * GNU-R's bytecode also map to the bytecode version and instruction metadata. + */ + static class Factory { + private final ImmutableIntArray posMap; + + private Factory(ImmutableIntArray posMap) { + this.posMap = posMap; + } + + /** Create a label from a GNU-R label. */ + BcLabel make(int gnurLabel) { + if (gnurLabel == 0) { + throw new IllegalArgumentException("GNU-R label 0 is reserved for the version number"); + } + + var target = posMap.get(gnurLabel); + if (target == -1) { + var gnurEarlier = gnurLabel - 1; + int earlier; + do { + earlier = posMap.get(gnurEarlier); + } while (earlier == -1); + var gnurLater = gnurLabel + 1; + int later; + do { + later = posMap.get(gnurLater); + } while (later == -1); + throw new IllegalArgumentException( + "GNU-R position maps to the middle of one of our instructions: " + + gnurLabel + + " between " + + earlier + + " and " + + later); + } + return new BcLabel(target); + } + + /** + * Create an object which creates labels from GNU-R labels, by building the map of positions in + * GNU-R bytecode to positions in our bytecode (see {@link Factory} implNote). + */ + static class Builder { + private final ImmutableIntArray.Builder map = ImmutableIntArray.builder(); + private int targetPc = 0; + + Builder() { + // Add initial mapping of 1 -> 0 (version # is 0) + map.add(-1); + map.add(0); + } + + /** Step m times in the source bytecode and n times in the target bytecode */ + @CanIgnoreReturnValue + Builder step(int sourceOffset, @SuppressWarnings("SameParameterValue") int targetOffset) { + if (sourceOffset < 0 || targetOffset < 0) { + throw new IllegalArgumentException("offsets must be nonnegative"); + } + + targetPc += targetOffset; + // Offsets before sourceOffset map to the middle of the previous instruction + for (int i = 0; i < sourceOffset - 1; i++) { + map.add(-1); + } + // Add target position + if (sourceOffset > 0) { + map.add(targetPc); + } + + return this; + } + + Factory build() { + return new Factory(map.build()); + } + } + } +} diff --git a/src/main/java/org/prlprg/rds/RDSReader.java b/src/main/java/org/prlprg/rds/RDSReader.java index 4ecb5ed5b..f2bdf1268 100644 --- a/src/main/java/org/prlprg/rds/RDSReader.java +++ b/src/main/java/org/prlprg/rds/RDSReader.java @@ -145,14 +145,9 @@ private BCodeSXP readByteCode1(SEXP[] reps) throws IOException { default -> throw new RDSException("Expected IntSXP"); }; - if (code.get(0) != Bc.R_BC_VERSION) { - throw new RDSException("Unsupported byte code version: " + code.get(0)); - } - - var bytecode = code.subArray(1, code.size()); var consts = readByteCodeConsts(reps); try { - return SEXPs.bcode(Bc.fromRaw(bytecode, consts)); + return SEXPs.bcode(Bc.fromRaw(code.data(), consts)); } catch (BcFromRawException e) { throw new RDSException("Error reading bytecode", e); } diff --git a/src/main/java/org/prlprg/sexp/IntSXP.java b/src/main/java/org/prlprg/sexp/IntSXP.java index 7986c6c78..e3c88d753 100644 --- a/src/main/java/org/prlprg/sexp/IntSXP.java +++ b/src/main/java/org/prlprg/sexp/IntSXP.java @@ -6,7 +6,7 @@ @Immutable public sealed interface IntSXP extends VectorSXP { - ImmutableIntArray subArray(int startIndex, int endIndex); + ImmutableIntArray data(); @Override default SEXPType type() { @@ -20,12 +20,8 @@ default SEXPType type() { IntSXP withAttributes(Attributes attributes); } -record IntSXPImpl(ImmutableIntArray data, @Override Attributes attributes) implements IntSXP { - @Override - public ImmutableIntArray subArray(int startIndex, int endIndex) { - return data.subArray(startIndex, endIndex); - } - +record IntSXPImpl(@Override ImmutableIntArray data, @Override Attributes attributes) + implements IntSXP { @Override public PrimitiveIterator.OfInt iterator() { return data.stream().iterator(); @@ -58,15 +54,8 @@ final class SimpleIntSXPImpl extends SimpleScalarSXPImpl implements Int } @Override - public ImmutableIntArray subArray(int startIndex, int endIndex) { - if (startIndex == endIndex && (startIndex == 0 || startIndex == 1)) { - return ImmutableIntArray.of(); - } else if (startIndex == 0 && endIndex == 1) { - return ImmutableIntArray.of(data); - } else { - throw new IndexOutOfBoundsException( - "subArray of simple scalar; startIndex=" + startIndex + ", endIndex=" + endIndex); - } + public ImmutableIntArray data() { + return ImmutableIntArray.of(data); } @Override @@ -83,13 +72,8 @@ private EmptyIntSXPImpl() { } @Override - public ImmutableIntArray subArray(int startIndex, int endIndex) { - if (startIndex == 0 && endIndex == 0) { - return ImmutableIntArray.of(); - } else { - throw new IndexOutOfBoundsException( - "subArray of empty vector; startIndex=" + startIndex + ", endIndex=" + endIndex); - } + public ImmutableIntArray data() { + return ImmutableIntArray.of(); } @Override diff --git a/src/main/java/org/prlprg/sexp/SEXP.java b/src/main/java/org/prlprg/sexp/SEXP.java index cb92adb2f..372555822 100644 --- a/src/main/java/org/prlprg/sexp/SEXP.java +++ b/src/main/java/org/prlprg/sexp/SEXP.java @@ -15,6 +15,11 @@ public sealed interface SEXP } /** + * Returns an SEXP which would be equal except it has the given attributes instead of its old + * ones. If the SEXP is a {@link RegEnvSXP}, it will mutate in-place and return itself. If the + * SEXP is a list or vector containing environments, this performs a shallow copy, so mutating the + * environments in one version will affect the other. + * * @throws UnsupportedOperationException if the SEXP doesn't support attributes. */ default SEXP withAttributes(Attributes attributes) { From 1a1f31ef364043313b92c201ecc052723f775b18 Mon Sep 17 00:00:00 2001 From: jakobeha Date: Sat, 3 Feb 2024 20:34:01 -0500 Subject: [PATCH 09/12] add package-info --- src/main/java/org/prlprg/server/package-info.java | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 src/main/java/org/prlprg/server/package-info.java diff --git a/src/main/java/org/prlprg/server/package-info.java b/src/main/java/org/prlprg/server/package-info.java new file mode 100644 index 000000000..2e043e1d6 --- /dev/null +++ b/src/main/java/org/prlprg/server/package-info.java @@ -0,0 +1,9 @@ +/** Socket setup and communication with clients, and protocols. */ +@ParametersAreNonnullByDefault +@FieldsAreNonNullByDefault +@ReturnTypesAreNonNullByDefault +package org.prlprg.server; + +import javax.annotation.ParametersAreNonnullByDefault; +import org.prlprg.util.FieldsAreNonNullByDefault; +import org.prlprg.util.ReturnTypesAreNonNullByDefault; From f4099d67a77835016741d76bc9c449f673092a6b Mon Sep 17 00:00:00 2001 From: jakobeha Date: Sun, 4 Feb 2024 00:06:55 -0500 Subject: [PATCH 10/12] rename NotImplementedException to NotImplementedError --- .idea/settings.zip | Bin 21871 -> 22321 bytes src/main/java/org/prlprg/rds/RDSReader.java | 4 ++-- .../org/prlprg/util/NotImplementedError.java | 11 +++++++++++ .../prlprg/util/NotImplementedException.java | 11 ----------- 4 files changed, 13 insertions(+), 13 deletions(-) create mode 100644 src/main/java/org/prlprg/util/NotImplementedError.java delete mode 100644 src/main/java/org/prlprg/util/NotImplementedException.java diff --git a/.idea/settings.zip b/.idea/settings.zip index ca768a5a93734a33481db5c852ee1e4031426a81..fcb2ebabcc7e21d85caef552e437683617359626 100644 GIT binary patch delta 5383 zcmZWt2{@G78y|x(mSODsHr8x0cCzo=Xpk^jqeL>+D`QfY$o^TfC6{d32d#F>zGS)T zDx{Dlq@oo6&*=VdZr}5q=bZPP-+RvQecyB5b7s~n$m*)e=+S266f6*GYHA3!P7i3M z;Uo%B=aG>g2n>>(z)AUdQoEI=ja2uj$`RBUBeYsO=2flo%T4?OUNZD1B(Ubj+@y*V zec$O?RyEb26bexvY}KNj+JR2F%*Wi`7&jcP4%Vs)Zq2y0LGIN<=O^chnXDa@cxFXdBIMM1y0<$G<5v3@atW^$YxcZRpdH0p@_~}eYy4euT3vzpL@|(=o%OpMFtC7e8QKA8?05@hRP_VED9AC6aLh# zHFUM6oMu7l-w^D>+ZG`wcU&wrn#Lcwhp0HCgKD!XVk|dKO+mEO>f2_k|Xbfi=qz4sSD+WP(J~&WG>^l)rTi_=Q{HjT zA2t+W7OP*%1cj@POZpqPJbYkG-^ZP>Yv`!kmnawqqbM!YP6uDMfJx+7@3<(;{?#ySz8jp==-Vr&))PN_Szw29|9E=xB6*PG%_t%37G@-e- zTu6>s8VrNYsmEt9kX@x2R4DjDKz+{SP~O?In^2=1fbk;2sCC6+(T+AL!SHwV7p>{l z3}YyEsrDsj-v(EDb}-bkI;%*qRGPzN=*fuw9XP36x@sQXb{3TpTrIV#PkN|t6d5Kfcy*ZaJcnH^+Ca7N#z%tzZQ zQJ1uZJ}SSp8ZUhhD}`A6UX_-m<18|!>6{&d9#gKz&l9*MAdeuFA{nqdF@oQUSvwY%Z^ zRBW|q6sIkNq}p+nQWS7X}myx>p>=$Y>)$BnEaiBcaJ3Lk?K^E!qtVT* zo!s}l-yH8eaymVKCEmFuBU3)$Z1azj1lmfw2|*iYT+2|vZt&Y5j##eSg(#jUiMy@i zY3!M!G@tNaHEWt=7djF`|L{&U#YcU+6-19AZmZ2rIz54X-TEFU+%ku#&|WdaRn(N* zXv#3{MXVPDY!}^oMnISvZVv3%KRBinT}^jy&8%)^*mSqP1*YZrO}&*lWcUv;Oy<1Y z`A0H4T_qbg?Tgftof$g{n(sJN-F8u0@L^~h8UI8zzjlJdz|<>*&hUCj-#zECxHY^| z(X+7Iw9nEoonk2y^UoF(Rwom7=C+n+T08HjB&K442T>LpS#pHii%ri72n2WVXuwf>{tu`h#UAzOX080{PnF^ zB7WU|mif*Tew4xW{?_W?&-`dJDr$bOvu|^cK_C|`Adn-s9C1pA;qZoYR^lXPQTz!d zU2S!GoPfGa45`gaHcqO$6mR{MIRScTA*t<;%08&;=;=w55bL0nhqE0_K#Pb#`LCdq z<>ch$6r+|KMq?ijC-ochzt{vhP*IP3j~}@Tfk67ea)c9L#dL=>JA)(_(8(D0b0too zNbCuzZNgASgvUhnU9nfE1>Hoitx*jX!;VfjC`k4H1bh<>aRzp4b& z&<^y~+iDEUoXC0qa)U*l&?ScpH^1tq>tWQBJnK&F#51S^(J*$o9N*2+WNG!SN&Gm4 z-!!@9);8QOzY(r`4O+&M!AbjQwtV7AGn!E}n`O};$K*u4Ndt8o&wUu*~9t zrn?cFzto>0TZ~CpJ<0}=rD`!JS)_Ixtt#neNlO!U}xzG_V?Xw>QRy^m_stXweLQDRiz}geQ zQ2+nq0b72T126bp3Xt`14Rm#L4LFB67Z`Zql2?FCsISl3X{_H%Fj(s~H+W1D0>yY9 zq(NCO$|ARmV!33ZQa-LQHBIFveFNp*ZZ!=*4JDiHz0f&{&E}Rxo?R!$IEyzb8(5)m zwpUK#@oV*Ro=k~R2tsO#+LADnhqlVvDZ%gOyHe!~X<}b6JZjOcr--M(BRqRFa*_lQ z07W0A#lS71B;i}!1cClS!PSVFQH;=MombM0um3dG6gS2b`GU%gq$?H?D`xQ8_GHGO+ug!9Kk8>a3e?SH zMdsKg%Y|Qn{-XSylSaUjR9f4SylsrX1YR3u}aL2uW0YT zQb&_H-It<^>n-nMAIAjtaKKw)<@-O*B3>{Erl7M#UUM-O!+(fTMGDW)ElYDz>A<6U zx3%0wGnH*(+p(Wl1v)Zrbl^18KUgWB^NPFfZEVu6P*)#Dz)$F(Aro*V&q=P4Pd`q^ z3%znCDF?Eg%%`B2{k;o|=wGwE{xF*gH|a0$+gY@RKL1jtwxszQv8wrIebscZS9o3h z)2YeHx1BB9lP%jl0xMzb#no98@Im6{x_9`qaG9dcpmv6Rc` zCGu61ylUDZW%;)Km1{m8Us)lMp9|UtqkYd2xc8t3dlviGLs<_f1foy(pFIosqd1At z2BC*}*izLTv{UIhff>3CQa6KS39055EkCG<{qjZs<{ty8deR`jGAF_F z!X=M`J)W349N(c$7rTD4d!&eJ7wTS^-WTTI%~XSUy87Y4Rx6*J?vxdq09FyvTG+CuT zKB;(4X@r@oH1I}9bCX@A^!Z}3LUP{-9lx(>4k8~T>dS2(pI$SSzTf}Kej;~aK3}tD z;jCi8%9nR4RG%CzymE{)v%Zn-*Ai7D`sRhZd6KYdPu#D+;56cx9Xj`QyUSasf1m#( z`GE@~-@QRR1JCq2c=V6vbI^srh-5X%`U7{4BlMX`!k{uQa7?d^)DDW9rkdJMid6lP)w$mWDC4L5X#y1m0Szo@(>&Cp?c zbw=auFc(UFgk6Vmj(VP;$Z+49V_?LshN3hq%It188B#jR&B54^6`7aDgEJkA`|R`5 zQPX{l6OdrhU-lCZ{o0G8j0&)}*x6GXn6ou(;M$c$XP4o27(d1(Vl>L82uaP3U3;IH z_YXWid3+DM0D3&@zaF>$nPdTKBU!=|+?&+-Rr$QzwQZ(1Ixf8)+cl_a8#IX;uI=ra zR&yG47o@dj4ysMHX@cfa3s$S5BD;fq;u6NfUDDsbDs#P?5gis<_+!J{VWOA84^s+- zeNf{fZyFlTyE(9xVoMy^Okmfj_GZ;Y;gtVg$k5v&147APe@Mam8&+BFHl&v{4mX2|Wwv8Fq6=bqO@imKA z$?xt%#JnjTi%kcu@U@t-M{?10^6V?cffw}Rt`k0i2FLyM0t^D7 zh8+BYKlB11V~i3wkajHu+JZsMMS2K?<)?%Gseo&yLL^WLGi8$t*n15u9RG}x>IY;% zD_WEU%2iwmjRZRncKgwp&O(798aCjiDIE#;>$8jN$ZUe-fH_8-1hVt$O&A0R8$oVt zM?e%XARU7wZRQ~pZYV1)kVqi_(Egmfcf@bvG>G>F3-1xVF?!&H3_rktK4g)|=7op@ z0S6B)fg`{m20)5T2#z)*gR+2@{PWL8pEWns91bj6OOOa2^We<)1N9Pr(_e%Gf+jK~ zf;ym1F(Ysl#VPfl>FhbiQX^VHT^MMf@R9YknNoQ~e#1i*s5bgR+!~07AHky$fS(C3 ziPW#J$D`KUGXrD@et_PBjzlcgoEs|40#us|kVG?3e98ixHy0&|-z>PHKCA${r3guU zY{m`MX9K3p4#B&ic$Wje9UW{2MY^)-YzVvRiy;4jVsT(+c>KiHqYUO@+Mqkq30(7^w`lmIOo4ibxekVT0A zaK(W0|16Th&7vj*RGRRU5L_UseIdY%ivPDrdxIAB2?McaCrC(wB{%d665y5O`VHv{ zItnQQL|7bdS{KOl1q(1f1WE|x_`giS)`$r3QeOHueQPVwcVfULD*;mYe!VOhkLc@* z0}L2G5=i0Yh6ZQ?Qy3JL-D- z`s_f`&lq9R+)#ThAPg-+TEw1k1`_~Io(h(u$FENZ;8ElN9$O#AHvbeiG*TCsRu)gQ zISlReZOjP@&!z~2^{UB%nq#Wn|%|*`O0s{s> z_$fY;VmWA7m?7}g;slB3ugRYI-<80YrV&tWB}fAOnhU`+_si#D4D8!*A2PD#h5{yl Tnynav97GOc0Ro3i4xs-7Y+`pA delta 5172 zcmZu!2|Scr8=o0FjjgecosoTzrL5WaWiXbq?<7Q)5`&^-i}%vC?~;8Qu0)oQRCWp_ zTO#olQnKCeo$-C$)P3jo{LXpLdH&Dxf6kfr9DFVGQzaB`ZbU|ofKXFYL%w*b5looi zP%^Ls#L|zE+Aj1yB;qCcfk0jU?qX zaQHgxF3G?Sr}v=&2WAc+m*Wu$XTrn*d=|_gDPUL~4~^YtA0SPRB1%XthLr;_K-?hp ziH@R9M-> zgwV5y{BV95~ME*%NpV1jA{zKGSbu^UhclgW02W~`#fJAiJQIX5;i?wHCAst<$$0|@l|BOZJ!Yj?D98^ zyDrBn%FOKKqUi^|i7<)7na);0ggfP2tv7PgKk8AqI?VW*yuIpOI)r@TRe-Mf6cVkQ zjQSEkS1~^J`c;kR-DxG(odSM7h12ER8Ak29n|fEF#2T(7-H-5tLLj_k#2V85v_65G zLgl2|5Nj16T1)Cn=~VovSMG`Xq`sk^46f>;brY41zI=|$y)+?#2wJ;4^qtv5D^Y)2 zi0Fu@&1^*88`x=4?37h>wc`amA*Toa1eWh?NO zGSgz7O>wB_-$k`brXkwuu*L2d%HDFcyG7kXX2gBNH&sdG;^ujYEU~>#idzC#`5lm{ zBP-n&90VU=3I68~UTn2TiG6~Hdcff(>w%Is7hxJza=0z=j9Db@R~ikZ){fy2jng{Z zsu&RtU_&RJG#9PP4OD3kllmADeq2V{;4PTUu^XS%bg~j>U6v>wlF245_ppdNqQ)w? zcT;_owzosy=r}Cjzhb1;#!Ev(^{Fxa%(F?~ZuUC^;aIpLxTY3=9L$ceSdR&;{+06hv<4&Fs&<06E`Qh_H?oiC4J+?hITU3m! zV!2#tL_G|Wdo6w|^jZ$mW8MN+Ly^b&d_p2OEi&il>`U)7LbjtUu$B9WE^6vm&iZ+XtR=PTW)B>Wz++9bK7OU)%FE*)Li4#~ zbrM?^>$-%tZ`*d`oA4rmUmmfkoqnr@bz01~=E^01QmFwYi=@d+KzpHuY*%UN2H(H4 z3sPxS9$QHvn5N*HAE_-ARGz6z=n|j)BSry{A%vRA&07>+l)o-WKtmU&{?LvSzFRRQ z#OrS6lfog0BQwb`E+ylBmPA{Duk3hgnV^+r*TR6t!n4mhkv*Q$IC!GC)=`!p_?U&9 ztYS@uGX0d5@#O7GX_uw-&Xn}#D2aBVeB2%`Fol|1pRVtz!|Om|Wb}DsNDF>NGMO=m-;TI%!iVAkJ(> zeO$B33HQMLr6jIeg^qFrJE?7c_mOCislWi|Ey{20`G7Zs6<d3gf=T@Kl>JiU!12)TOs6dgeOIk$chfnJYJTKd203J6)k_EnxM7xR)yum%@nj3Gan-CrljFMb^I=GD239*h3 z!V$6ySX7F!u9orR$20~f9!j zIM27zSW<;-~x530g^*0uvr6Z`d-b)z=6fT{8s}-QZ)pVDZ%&*1TS?1dL-kj<% z#hhKJ=&JhI7qN4pvx;k0xcmj0wty+gFRX;}NLsn+n~U-(ABVO+Ni_M?-OTrFS%2gx zrIG_rf$fZ~K2dxW)V`kGuEwR(NYkbJ3Q2LmMfI-v;^zWY*`A_JwEq-g`Ly;E_Z72j zoEdw$MasgtB-51S)P=}s$yZyW=a!n2ug;CK2wDV~4$F70a&3-#_KFQ( zKWF#sTns(BhiWLqWEH6-an|KSOw&nBeG<;r{IUrR2kJs(HvNR4Z-XsVCs#K@oq)+M zZ)W@tEf=lqa`$$a(B94If!#MJ9yXQP3EdYDrTF-j;(X&5Uxls8rmdBbH`yPW$5$W9 zkB3zaU#^;rsG2aX8a1zaYf&|2_53g8hMVZj>AG!I?etl`g0`yT9T`$a?ZR986l&L8 zakW(k+~qRQB)+q}r;@3qO!FPG+Rym@@WqWkWBb>35u=&$Y$EUo^%r|izJ2H?%lcTc zvHPPbJy#lB9rDkW1bq_Rvl~kb7>_V|P{XQDaiF4hQHWb`j=eh#ms63H?r^qTU74*W zm!-2&LVnzw5+7CT#vW`KX{6gwtQm&O3d4Q(<#i}GC`W`TbQy|`2UW=N2+z7t9{VrEcRoEV% zztjB_lUB2rhG=1&$Mc7sgm|vSB?4!b2l-RlEbH_#gB7Tjhh7MLy%dp@0`s`dQ&NIk zefF~FTh_<8g6i$J0)skkllO+h8phcgLf+it=;N+W#6_j5Ra{6aZR+;;+8vzLzxgKg zrpld|xu+dp9P>DZC0@tU&`a$sJY^Z|QO~FLdt_6<`JzOWf76VMZNE!Gd{uI^!E5*A zKsIgx#ua3+a`5gut8BV*qkF`pv68%;Demv(Eyp6=(PgTF_ED;d#{7K>nSxxJtb=Ni z0?O-_t189@iE|o06RzRX@h8Q@16x_Ux@5-oFVii|Nq>#h+_Ou3H>WS)N+GGN01^5! zW%H2I`H#<$IVtejipI(%b3WwNX8#!`OUA*PzV3CNDC=l5vPag&Jk-wOf(*U&EVl!w zv?dID$)J8Z3q{m_1>qI0D`U>1W`|uOE=b;-akZvDIlt>a5!=Y5k`pcv*nQku(ec4p z#vP?5-1``_y_TN+VShX82QaIznwBaL_EJ632V}%=zRIX<=~F5QB-Hws^Av%Jmj3XJ z6s_7rI`a^3At9Wx#6Ce^jr0EpC=S94zVO4Bi~5QwG7zIV4;!HBLg?WUN!yVv;UU;QE92L*%T=^(a}>;22CfLrHvwHS5Y;T*_#b z5B7xeBa?huk?>{jV-rg1!BVolTGsuVOYpjI$8oVM-tUGIu4ue&sXdr+T7BjGhL?wq zT6elJ9(O|S@8P~x4+&`u>m!YCf5!!D>e*x%6zgh8cImdUM{N%1m*>L7t7WfdGb9ui zuP^KPWMpl93oNp(*Qp%a8=+E;tEAKVP|#Fd&M>fMw%}jXYuC+2WD0K1WV8mJjza!- zI--R@s3FAbc#M38u{;V;OWFii-kETVng9Egnx*+h;fCE69oFMV|%m@i%gaT>w z0sxKiG18EWF&7jHL>Wst5TUS>RG*)N&@~XsKs5T}DTP5G0fCo%0{nx~p>Dxa;TQbO zji4|D_a3 zXmpIENXZi}6b&l0fx->SKM$G;;KhiOqMtTE!gi>D9RmRpCP)5bcnql63ku>N0NA@v z1H>8%kTCUTNLUCh&~GMmh8R$u%ih&oP$ToVlxOMwN!icE^IuX1OfX1T^D)2|BSZ4v zOuj$K4<;85R*3nCe_uG@%!&Z|Ob?TD=pN}H2HFgYz!Amg3_vHl5Wrz}SRS}35|+yd zh?xqLV(&LW!ZewH6%#&^zn?GPM>$$#0t!sTNlI5E!YPo)A&`w(2!!p3uUcjREe782 zEf0gbT|wFA4l3LNh4Y9ay)Xc^5FuG}+E&jB0w+^J5j>*wP#8d4AM(&p^ce$ylW>sJ zqb?&)0D-0`lBJ(-okz*SiU3UJe58c)L4u)TKnq5ggppKZSB?V>{|A(#hAYJYK1&n{ zlZ-{eE{OxGI>_Ia=tvl3BL&3vfpXL*uf#va_VT&p9pd^g^|t-_4hT~E)3YW4L|gNd z{Nun~#v%oLg7X3`Mu%lI0Yhq%0(`NDb*poO_)CK8q@VoPG|JI{hSgyjyoN{^yDXq$ zcvyM~3nZ*f7Fb0|GW@dffI4F&%v}!XVC491%Et-`gUAE%Rzf8Ha_hVsEx@SlK|%f$ zJp|Gu4|p?h|AvQxfmpeTSCc}NZ1=q zAXQaF;@99pJj$~z!K55PIl7xlwSe=SyuaC+18r$)0|%CZq`afdkuWD6AjkZ0egFKg n4L`!wrY_KFjU-`atdTIJK2T@CkD#C=Dxtw28K`cpPkj3?{&@^H diff --git a/src/main/java/org/prlprg/rds/RDSReader.java b/src/main/java/org/prlprg/rds/RDSReader.java index f2bdf1268..8a5375897 100644 --- a/src/main/java/org/prlprg/rds/RDSReader.java +++ b/src/main/java/org/prlprg/rds/RDSReader.java @@ -17,7 +17,7 @@ import org.prlprg.primitive.Constants; import org.prlprg.primitive.Logical; import org.prlprg.sexp.*; -import org.prlprg.util.NotImplementedException; +import org.prlprg.util.NotImplementedError; public class RDSReader implements Closeable { private final RDSInputStream in; @@ -116,7 +116,7 @@ private SEXP readItem() throws IOException { PACKAGESXP, PERSISTSXP, CLASSREFSXP -> - throw new NotImplementedException(); + throw new NotImplementedError(); }; }; } diff --git a/src/main/java/org/prlprg/util/NotImplementedError.java b/src/main/java/org/prlprg/util/NotImplementedError.java new file mode 100644 index 000000000..f4a743f0b --- /dev/null +++ b/src/main/java/org/prlprg/util/NotImplementedError.java @@ -0,0 +1,11 @@ +package org.prlprg.util; + +public class NotImplementedError extends Error { + public NotImplementedError(Object switchCase) { + super("Sorry, this case is not yet implemented: " + switchCase); + } + + public NotImplementedError() { + super("Sorry, this code is not yet implemented"); + } +} diff --git a/src/main/java/org/prlprg/util/NotImplementedException.java b/src/main/java/org/prlprg/util/NotImplementedException.java deleted file mode 100644 index c82879ff5..000000000 --- a/src/main/java/org/prlprg/util/NotImplementedException.java +++ /dev/null @@ -1,11 +0,0 @@ -package org.prlprg.util; - -public class NotImplementedException extends UnsupportedOperationException { - public NotImplementedException(Object switchCase) { - super("Sorry, this case is not yet implemented: " + switchCase); - } - - public NotImplementedException() { - super("Sorry, this code is not yet implemented"); - } -} From ca8ad4b2ccc43a4ccf3180db7ac5cea5d6c95a58 Mon Sep 17 00:00:00 2001 From: jakobeha Date: Sun, 4 Feb 2024 00:08:00 -0500 Subject: [PATCH 11/12] add documentation to `SEXPs` that it has all global methods to create SEXPs --- src/main/java/org/prlprg/sexp/SEXPs.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/prlprg/sexp/SEXPs.java b/src/main/java/org/prlprg/sexp/SEXPs.java index 041404745..9421f066d 100644 --- a/src/main/java/org/prlprg/sexp/SEXPs.java +++ b/src/main/java/org/prlprg/sexp/SEXPs.java @@ -12,6 +12,7 @@ import org.prlprg.primitive.Constants; import org.prlprg.primitive.Logical; +/** All global SEXPs and methods to create SEXPs are here so they're easy to find. */ public final class SEXPs { // region constants public static final NilSXP NULL = NilSXP.INSTANCE; From 4643e2bae167eba3b8638f11709b4289060366a2 Mon Sep 17 00:00:00 2001 From: jakobeha Date: Sun, 4 Feb 2024 00:31:06 -0500 Subject: [PATCH 12/12] add server draft skeleton and simple unit tests --- src/main/java/org/prlprg/RVersion.java | 52 ++++++ .../prlprg/server/ClientHandleException.java | 14 ++ .../java/org/prlprg/server/ClientHandler.java | 120 ++++++++++++++ .../server/ClientParseViolationException.java | 18 ++ .../java/org/prlprg/server/ClientState.java | 35 ++++ .../server/ClientStateViolationException.java | 21 +++ .../org/prlprg/server/ReqRepHandlers.java | 57 +++++++ src/main/java/org/prlprg/server/Server.java | 155 ++++++++++++++++++ .../server/SomeClientHandleException.java | 13 ++ .../java/org/prlprg/util/WeakHashSet.java | 84 ++++++++++ .../org/prlprg/server/ClientStateTests.java | 17 ++ .../java/org/prlprg/server/ServerTests.java | 102 ++++++++++++ 12 files changed, 688 insertions(+) create mode 100644 src/main/java/org/prlprg/RVersion.java create mode 100644 src/main/java/org/prlprg/server/ClientHandleException.java create mode 100644 src/main/java/org/prlprg/server/ClientHandler.java create mode 100644 src/main/java/org/prlprg/server/ClientParseViolationException.java create mode 100644 src/main/java/org/prlprg/server/ClientState.java create mode 100644 src/main/java/org/prlprg/server/ClientStateViolationException.java create mode 100644 src/main/java/org/prlprg/server/ReqRepHandlers.java create mode 100644 src/main/java/org/prlprg/server/Server.java create mode 100644 src/main/java/org/prlprg/server/SomeClientHandleException.java create mode 100644 src/main/java/org/prlprg/util/WeakHashSet.java create mode 100644 src/test/java/org/prlprg/server/ClientStateTests.java create mode 100644 src/test/java/org/prlprg/server/ServerTests.java diff --git a/src/main/java/org/prlprg/RVersion.java b/src/main/java/org/prlprg/RVersion.java new file mode 100644 index 000000000..0cb3d1d0c --- /dev/null +++ b/src/main/java/org/prlprg/RVersion.java @@ -0,0 +1,52 @@ +package org.prlprg; + +import javax.annotation.Nullable; + +/** + * Major.Minor.Patch version number with an optional suffix for things like "Beta" and "RC". + * + *

This class needs to support whatever format GNU-R versions can have. But it's not a GNU-R + * specific + */ +public record RVersion(int major, int minor, int patch, @Nullable String suffix) { + /** The latest version we handle. */ + public static final RVersion LATEST_AWARE = new RVersion(4, 3, 2); + + public static RVersion parse(String textual) { + var parts = textual.split("\\."); + + int major; + int minor; + int patch; + try { + major = Integer.parseInt(parts[0]); + minor = Integer.parseInt(parts[1]); + patch = Integer.parseInt(parts[2]); + } catch (ArrayIndexOutOfBoundsException e) { + throw new IllegalArgumentException("Invalid version number: " + textual, e); + } + + String suffix; + if (parts.length > 3) { + if (parts[3].startsWith("-")) { + suffix = parts[3].substring(1); + } else { + throw new IllegalArgumentException( + "Invalid version number: " + textual + " (suffix must start with '-')"); + } + } else { + suffix = null; + } + + return new RVersion(major, minor, patch, suffix); + } + + RVersion(int major, int minor, int patch) { + this(major, minor, patch, null); + } + + @Override + public String toString() { + return major + "." + minor + "." + patch + (suffix == null ? "" : "-" + suffix); + } +} diff --git a/src/main/java/org/prlprg/server/ClientHandleException.java b/src/main/java/org/prlprg/server/ClientHandleException.java new file mode 100644 index 000000000..844fab6d3 --- /dev/null +++ b/src/main/java/org/prlprg/server/ClientHandleException.java @@ -0,0 +1,14 @@ +package org.prlprg.server; + +/** + * An exception a background thread got when processing a client request. This exception is checked + * while the underlying exception may not be. + */ +public final class ClientHandleException extends Exception { + public final String address; + + public ClientHandleException(String address, Throwable cause) { + super("Handler/thread for client " + address + " crashed", cause); + this.address = address; + } +} diff --git a/src/main/java/org/prlprg/server/ClientHandler.java b/src/main/java/org/prlprg/server/ClientHandler.java new file mode 100644 index 000000000..ecbd4d9b6 --- /dev/null +++ b/src/main/java/org/prlprg/server/ClientHandler.java @@ -0,0 +1,120 @@ +package org.prlprg.server; + +import javax.annotation.Nullable; +import org.zeromq.ZMQ; +import org.zeromq.ZMQ.Error; +import org.zeromq.ZMQException; + +/** + * Manages communication between the server and a particular client. Contains the client's socket, + * thread, and {@link ClientState}. + * + *

This class doesn't implement {@link AutoCloseable}; if you want to close this, close the + * socket. The reason is that the socket may close on its own, so we have to handle that case + * anyways, and don't want two separate ways to close. + */ +final class ClientHandler { + private final String address; + private final ZMQ.Socket socket; + private final Thread thread; + private @Nullable Throwable exception = null; + // When the socket closes, we aren't made aware until we handle its next message and get null or + // an exception. + private boolean isDefinitelyClosed = false; + private final ClientState state = new ClientState(); + + /** + * This will immediately start handling client requests on a background thread immediately. + * + *

As specified in the class description, to close this (and stop the thread), close the + * socket. + * + * @param address The address of the socket. It must resolve to the same address as {@code + * socket.getLastEndpoint()}, however the actual text may be different (e.g. localhost vs + * 127.0.0.1) which is why we need to pass it explicitly (because otherwise {@link + * Server#unbind} won't work). + */ + ClientHandler(String address, ZMQ.Socket socket) { + this.address = address; + this.socket = socket; + this.thread = + new Thread( + () -> { + try { + while (true) { + var message = socket.recvStr(); + if (message == null) { + // Socket closed + isDefinitelyClosed = true; + break; + } + var response = ReqRepHandlers.handle(state, message); + if (response != null) { + socket.send(response); + } + } + } catch (ZMQException e) { + close(); + var error = Error.findByCode(e.getErrorCode()); + switch (error) { + // Currently assume these are normal + case Error.ECONNABORTED: + case Error.ETERM: + System.out.println("ClientState-" + address() + " closed with " + error); + break; + default: + exception = e; + throw e; + } + } catch (Throwable e) { + close(); + exception = e; + throw e; + } + }); + thread.setName("ClientState-" + address()); + thread.start(); + } + + /** The address the client is connected to. */ + String address() { + return address; + } + + /** + * Close the underlying socket, which makes the thread stop on next communication (if it's doing + * something it won't stop immediately). + * + *

If the socket is already closed, the behavior of this method is undefined. + */ + void close() { + if (isDefinitelyClosed) { + throw new IllegalStateException("Socket already closed"); + } + isDefinitelyClosed = true; + socket.close(); + } + + /** + * Throws a {@link ClientHandleException} if the client disconnected because it got an exception. + */ + void checkForException() throws ClientHandleException { + if (exception != null) { + throw new ClientHandleException(address(), exception); + } + } + + /** + * Waits for the client to disconnect and the socket to close on its own. + * + * @throws ClientHandleException if a client disconnected because it got an exception. + */ + void waitForDisconnect() throws ClientHandleException { + try { + thread.join(); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + checkForException(); + } +} diff --git a/src/main/java/org/prlprg/server/ClientParseViolationException.java b/src/main/java/org/prlprg/server/ClientParseViolationException.java new file mode 100644 index 000000000..0bbc14463 --- /dev/null +++ b/src/main/java/org/prlprg/server/ClientParseViolationException.java @@ -0,0 +1,18 @@ +package org.prlprg.server; + +/** The client sent a malformed message. */ +public final class ClientParseViolationException extends IllegalStateException { + @SuppressWarnings("unused") + ClientParseViolationException(String message) { + super(message); + } + + ClientParseViolationException(Throwable cause) { + super(cause); + } + + @SuppressWarnings("unused") + ClientParseViolationException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/src/main/java/org/prlprg/server/ClientState.java b/src/main/java/org/prlprg/server/ClientState.java new file mode 100644 index 000000000..363fa0a60 --- /dev/null +++ b/src/main/java/org/prlprg/server/ClientState.java @@ -0,0 +1,35 @@ +package org.prlprg.server; + +import javax.annotation.Nullable; +import org.prlprg.RVersion; + +/** + * State for a client stored on the server visible to {@link ReqRepHandlers}. + * + *

Doesn't include the socket and client thread, that's the role of {@link ClientHandler}. + */ +final class ClientState { + private record Init(RVersion version) {} + + private @Nullable Init init; + + ClientState() {} + + void init(RVersion version) { + if (init != null) { + throw new ClientStateViolationException("Client already initialized"); + } + init = new Init(version); + } + + RVersion version() { + return init().version; + } + + private Init init() { + if (init == null) { + throw new ClientStateViolationException("Client not initialized"); + } + return init; + } +} diff --git a/src/main/java/org/prlprg/server/ClientStateViolationException.java b/src/main/java/org/prlprg/server/ClientStateViolationException.java new file mode 100644 index 000000000..6e0fdbbb9 --- /dev/null +++ b/src/main/java/org/prlprg/server/ClientStateViolationException.java @@ -0,0 +1,21 @@ +package org.prlprg.server; + +/** + * The client sent a message or data the server wasn't expecting in its state. e.g. client + * initializes itself twice. + */ +public final class ClientStateViolationException extends IllegalStateException { + ClientStateViolationException(String message) { + super(message); + } + + @SuppressWarnings("unused") + ClientStateViolationException(Throwable cause) { + super(cause); + } + + @SuppressWarnings("unused") + ClientStateViolationException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/src/main/java/org/prlprg/server/ReqRepHandlers.java b/src/main/java/org/prlprg/server/ReqRepHandlers.java new file mode 100644 index 000000000..8a9861594 --- /dev/null +++ b/src/main/java/org/prlprg/server/ReqRepHandlers.java @@ -0,0 +1,57 @@ +package org.prlprg.server; + +import javax.annotation.Nullable; +import org.prlprg.RVersion; +import org.prlprg.util.NotImplementedError; + +/** + * Functions for each initial request the client can make. They take the initial request body as + * params (as well as client state), and return the final response (or void if there are none). If a + * request is more complicated than a simple function (e.g. server can respond with "more + * information needed" and then wait until the client sends more), the function will also take a + * {@link Mediator} to send intermediate responses and handle intermediate requests (possibly out- + * of-order, we're not sure what kinds of communicatio we'll need yet). + * + *

The specific handlers are actually all private, because this class's interface has the method + * which handles a generic initial request, parsing and dispatching to the specific handler. + */ +final class ReqRepHandlers { + private static final String SIMPLE_ACK = ""; + + /** + * Handle an initial request (not intermediate request in a request chain) from the client. + * + *

Apparetly ZMQ needs every request to have some response. If this returns null, it already + * send a response. If this only needs to do a simple ACK, it will return the empty string. + */ + // TODO: Add GenericMediator which can create all other mediators which we'll do depending on the + // request type. GenericMediator will have a reference to the ClientHandler's socket and thread + // so it can send and receive subsequect requests, + static @Nullable String handle(ClientState state, String request) { + // TODO: Parse request type and the rest of the data, create specific mediator if necessary, + // dispatch to the specific handler, and encode the response. + if (request.equals("Hello, server!")) { + throw new ClientStateViolationException("bad message"); + } + if (request.startsWith("Proper init ")) { + RVersion rVersion; + try { + rVersion = RVersion.parse(request.substring("Proper init ".length())); + } catch (IllegalArgumentException e) { + throw new ClientParseViolationException(e); + } + init(state, rVersion); + return SIMPLE_ACK; + } + if (request.startsWith("Something which returns null so IntelliJ doesn't complain")) { + return null; + } + throw new NotImplementedError(); + } + + // region specific handlers + private static void init(ClientState state, RVersion rVersion) { + state.init(rVersion); + } + // endregion +} diff --git a/src/main/java/org/prlprg/server/Server.java b/src/main/java/org/prlprg/server/Server.java new file mode 100644 index 000000000..4d997fa8f --- /dev/null +++ b/src/main/java/org/prlprg/server/Server.java @@ -0,0 +1,155 @@ +package org.prlprg.server; + +import com.google.common.collect.ImmutableList; +import java.io.Closeable; +import java.util.Set; +import org.prlprg.util.WeakHashSet; +import org.zeromq.SocketType; +import org.zeromq.ZContext; + +/** + * An instance of the compile server which communicates with clients. + * + *

All methods to send and receive data are here so they're easy to find. + * + * @implNote The current plan is for the server's interface to be exclusive to the main thread, + * which includes adding sockets and closing. The server spawns separate threads for each client + * to wait and handle their requests, and the clients threads may spawn child threads for + * specific multi-part requests (although first the clients need to be multi- threaded). + */ +public final class Server implements Closeable { + // TODO: Make this an environment variable, but we should have some centralized configuration + // static class with all environment variables + private static final int IO_THREADS = 1; + + private final Thread mainThread = Thread.currentThread(); + private final ZContext context = new ZContext(IO_THREADS); + // Assume that ClientHandlers get garbage collected when their sockets and threads close. + // This doesn't mean all client handlers in the set are open, don't assume so. + private final Set clients = new WeakHashSet<>(); + + /** Creates a new server. */ + public Server() {} + + /** + * Binds the server to the given address so a client can connect. Currently, we only allow one + * client per address, but a server can bind to multiple addresses. + * + *

This method must be called from the thread the server was created on. + * + * @param address The address to bind to, e.g. "tcp://*:5555". + */ + public void bind(String address) { + requireMainThread("bind"); + + var socket = context.createSocket(SocketType.REP); + var isBound = socket.bind(address); + // Does ZMQ ever return false without throwing? If so, we need to handle + assert isBound; + + clients.add(new ClientHandler(address, socket)); + } + + /** + * Unbinds the server from the given address so, if a client is connected, it will disconnect. If + * the client socket was closed, this method may or may not throw {@link + * IllegalArgumentException}. + * + *

If the client was once but already no longer connected, the behavior of this method is + * undefined. + * + *

This method must be called from the thread the server was created on. + * + * @param address The address to unbind from, e.g. "tcp://*:5555". + * @throws IllegalArgumentException If no client is bound to the given address. + */ + public void unbind(String address) { + requireMainThread("unbind"); + + for (var client : clients) { + if (client.address().equals(address)) { + client.close(); + return; + } + } + throw new IllegalArgumentException("No client bound to " + address); + } + + /** + * Throws a {@link SomeClientHandleException} if any client disconnected because it got an + * exception. + * + *

This method must be called from the thread the server was created on. + */ + void checkForAnyException() throws SomeClientHandleException { + requireMainThread("checkForAnyException"); + + var exceptionsBuilder = + ImmutableList.builderWithExpectedSize(clients.size()); + + for (var client : clients) { + try { + client.checkForException(); + } catch (ClientHandleException e) { + exceptionsBuilder.add(e); + } + } + + var exceptions = exceptionsBuilder.build(); + if (!exceptions.isEmpty()) { + throw new SomeClientHandleException(exceptions); + } + } + + /** + * Waits for all clients to disconnect. + * + *

This method must be called from the thread the server was created on. + * + * @throws SomeClientHandleException If any client disconnected because it got an exception. + */ + public void waitForAllDisconnect() throws SomeClientHandleException { + requireMainThread("waitForAllDisconnect"); + + var exceptionsBuilder = + ImmutableList.builderWithExpectedSize(clients.size()); + + for (var client : clients) { + try { + client.waitForDisconnect(); + } catch (ClientHandleException e) { + exceptionsBuilder.add(e); + } + } + + var exceptions = exceptionsBuilder.build(); + if (!exceptions.isEmpty()) { + throw new SomeClientHandleException(exceptions); + } + } + + /** + * Closes the server, disconnecting all clients. + * + *

This method must be called from the thread the server was created on. + */ + @Override + public void close() { + requireMainThread("close"); + + // Probably handled by context but anyways + if (context.isClosed()) { + throw new IllegalStateException("Server already closed"); + } + + context.close(); + // Sockets will disconnect, and client handlers will stop their threads, automatically + } + + private void requireMainThread(String method) { + if (Thread.currentThread() != mainThread) { + throw new IllegalStateException( + "Method " + method + " must be called from the thread the server was created on"); + } + } +} diff --git a/src/main/java/org/prlprg/server/SomeClientHandleException.java b/src/main/java/org/prlprg/server/SomeClientHandleException.java new file mode 100644 index 000000000..d0f66af7d --- /dev/null +++ b/src/main/java/org/prlprg/server/SomeClientHandleException.java @@ -0,0 +1,13 @@ +package org.prlprg.server; + +import com.google.common.collect.ImmutableList; + +/** If any client threads closed because of an exception, the server will throw this. */ +public final class SomeClientHandleException extends Exception { + public final ImmutableList clientExceptions; + + public SomeClientHandleException(ImmutableList clientExceptions) { + super("Some client handlers/threads crashed, see the client exceptions for details."); + this.clientExceptions = clientExceptions; + } +} diff --git a/src/main/java/org/prlprg/util/WeakHashSet.java b/src/main/java/org/prlprg/util/WeakHashSet.java new file mode 100644 index 000000000..4d319c7d3 --- /dev/null +++ b/src/main/java/org/prlprg/util/WeakHashSet.java @@ -0,0 +1,84 @@ +package org.prlprg.util; + +import java.util.Collection; +import java.util.Iterator; +import java.util.Set; +import java.util.WeakHashMap; + +/** + * Set version of {@link java.util.WeakHashMap}: all the keys are weakly refereced and when they get + * garbage collected, the corresponding value is removed from the set. + */ +public class WeakHashSet implements Set { + private final WeakHashMap map = new WeakHashMap<>(); + + @Override + public int size() { + return map.size(); + } + + @Override + public boolean isEmpty() { + return map.isEmpty(); + } + + @SuppressWarnings("SuspiciousMethodCalls") + @Override + public boolean contains(Object o) { + return map.containsKey(o); + } + + @Override + public Iterator iterator() { + return map.keySet().iterator(); + } + + @Override + public Object[] toArray() { + return map.keySet().toArray(); + } + + @Override + public T1[] toArray(T1[] a) { + return map.keySet().toArray(a); + } + + @Override + public boolean add(T t) { + return map.put(t, null) == null; + } + + @Override + public boolean remove(Object o) { + return map.remove(o) != null; + } + + @Override + public boolean containsAll(Collection c) { + return map.keySet().containsAll(c); + } + + @Override + public boolean addAll(Collection c) { + boolean changed = false; + for (var e : c) { + changed |= add(e); + } + return changed; + } + + @Override + public boolean retainAll(Collection c) { + return map.keySet().retainAll(c); + } + + @Override + public boolean removeAll(Collection c) { + return map.keySet().removeAll(c); + } + + @Override + public void clear() { + map.clear(); + } +} diff --git a/src/test/java/org/prlprg/server/ClientStateTests.java b/src/test/java/org/prlprg/server/ClientStateTests.java new file mode 100644 index 000000000..825522aab --- /dev/null +++ b/src/test/java/org/prlprg/server/ClientStateTests.java @@ -0,0 +1,17 @@ +package org.prlprg.server; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.junit.jupiter.api.Test; +import org.prlprg.RVersion; + +public class ClientStateTests { + @Test + public void testClientState() { + var clientState = new ClientState(); + assertThrows(ClientStateViolationException.class, clientState::version); + clientState.init(RVersion.LATEST_AWARE); + assertEquals(RVersion.LATEST_AWARE, clientState.version()); + } +} diff --git a/src/test/java/org/prlprg/server/ServerTests.java b/src/test/java/org/prlprg/server/ServerTests.java new file mode 100644 index 000000000..0dc4fc208 --- /dev/null +++ b/src/test/java/org/prlprg/server/ServerTests.java @@ -0,0 +1,102 @@ +package org.prlprg.server; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; +import java.net.ServerSocket; +import org.junit.jupiter.api.Test; +import org.zeromq.SocketType; +import org.zeromq.ZSocket; + +public class ServerTests { + @Test + public void testEmptyServer() throws SomeClientHandleException { + var server = new Server(); + server.waitForAllDisconnect(); + server.close(); + } + + @Test + public void testServerImmediatelyUnbind() throws SomeClientHandleException { + int port = getUnusedPort(); + + var server = new Server(); + server.bind("tcp://localhost:" + port); + + var aClient = new ZSocket(SocketType.REQ); + aClient.connect("tcp://localhost:" + port); + + server.unbind("tcp://localhost:" + port); + + // Client won't send + aClient.sendStringUtf8("Hello, server!"); + + // These will do nothing + server.checkForAnyException(); + server.waitForAllDisconnect(); + // Close should still work + server.close(); + } + + @Test + public void testServerWithABadClient() { + int port = getUnusedPort(); + + var server = new Server(); + server.bind("tcp://localhost:" + port); + + var aClient = new ZSocket(SocketType.REQ); + aClient.connect("tcp://localhost:" + port); + + // This isn't a valid message, so it will trigger an exception in the background thread + aClient.sendStringUtf8("Hello, server!"); + + // Which will propogate when we join + assertThrows(SomeClientHandleException.class, server::waitForAllDisconnect); + // And check exceptions (this must be called after join or the client handler may have not yet + // thrown anything) + assertThrows(SomeClientHandleException.class, server::checkForAnyException); + + // Close should still work + server.close(); + } + + @Test + public void testServerWithAGoodClientOnlyInit() throws SomeClientHandleException { + int port = getUnusedPort(); + + var server = new Server(); + server.bind("tcp://localhost:" + port); + + var aClient = new ZSocket(SocketType.REQ); + aClient.connect("tcp://localhost:" + port); + + // This is a valid message and will initialize the client + aClient.sendStringUtf8("Proper init 1.23.45"); + assertEquals("", aClient.receiveStringUtf8()); + + // So this will do nothing + server.checkForAnyException(); + + // This is now invalid because the client has already been initialized + aClient.sendStringUtf8("Proper init 1.23.45"); + // If we try receiveStringUtf8 here it will hang forever. Even though we closed the socket on + // the server end (and even we unbind), that's what ZMQ seems to do. + + // So this will throw an exception + assertThrows(SomeClientHandleException.class, server::waitForAllDisconnect); + + // This should work normally, as always + server.close(); + } + + private int getUnusedPort() { + // From https://stackoverflow.com/questions/2675362/how-to-find-an-available-port + try (var socket = new ServerSocket(0)) { + return socket.getLocalPort(); + } catch (IOException e) { + throw new RuntimeException("Failed to find an unused port", e); + } + } +}