Repository navigation
CSEC: Precompiled classes - #79
Conversation
Coverage report
Show new covered files 🐣
Show files with reduced coverage 🔻
Test suite run success1251 tests passing in 75 suites. Report generated by 🧪jest coverage report action from 1913928 |
|
Good progress. Test cases currently failing; can you take a look? |
|
Oops — should've marked this one as draft. Still quite a bit of baking left to do for this one. |
7345635 to
4d10a34
Compare
Address the second Codex review: - Push the reset instruction before calling the native handler, so control items the handler pushes run instead of being discarded by RESET. - Reject native methods that declare a body, in classes and enums, with NativeMethodHasBodyError (as in #79).
4d10a34 to
305eea7
Compare
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 305eea7bd7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9a41fb807
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ddace296c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
resOverride searched only the runtime class's own frame, which holds only the methods that class declares. With Base b = new C(), where B overrides f and C extends B, b.f() ran Base.f. Search the runtime class and then its superclasses, nearest first. Predates #79, but every class now inherits Object's natives, so e.g. Object o = new Dog(); o.toString() ran the native instead of an override in Dog's superclass.
* native methods * display native function descriptor when unimplemented * native method handling for type-checker * revert Object class declaration * Handle bodyless methods in enums; drop type checker console.log Address the Codex review of this PR: - A native method declared in an enum crashed typeCheck, because the extractor now yields an undefined body for `;` and only the class branch of the checker handled that. Give the EnumDeclaration branch the same native / MissingMethodBodyError handling. - Remove the leftover console.log of native method declarations. * Run native handler control items; reject native methods with a body Address the second Codex review: - Push the reset instruction before calling the native handler, so control items the handler pushes run instead of being discarded by RESET. - Reject native methods that declare a body, in classes and enums, with NativeMethodHasBodyError (as in #79). --------- Co-authored-by: hj235 <hongjing235@gmail.com>
Removed the expectation of an empty `Object` class from all tests, since that behaviour is now deprecated. A blank `Object` class will be synthesized instead by utility function.
Previously, the library classes would be injected but the evaluator would not run the program to completion as it was limited by the input target step.
The frontend calls runECEvaluator(code, targetStep); without callbacks, output goes to console.log and console.error.
- println: unquote string literals and decode simple escapes, so println("hi")
prints hi rather than "hi".
- toString: return a quoted string literal like any other string value, use the
unsigned hex of the hash, and name library classes by their binary name
(java.lang.Object@...), as Class.getName() does.
- LFSR: keep the state non-negative and use Math.imul in FNV-1a. The sign bit
was smeared in by the shift and the register saturated to -1 for about half
of all seeds.
One file per library class (Object, System) plus library loading: hashCode, toString, println, and the LFSR, including determinism across re-runs to every step count. it.todo marks known gaps.
Revert the rename of Closure.mtdOrCon to Closure.decl. The frontend's CSE machine reads closure.mtdOrCon for method tooltips, and the renamed field made it throw, so nothing was drawn. The field still admits NativeDeclaration.
typeCheck() can throw, for example on a record declaration, and it ran before the try block, so runECEvaluator threw instead of resolving to an error result. Library loading moves inside the try as well. Type errors were also printed with console.error after going to the stderr callback; the callback already falls back to console.error.
…Context's code - PrintStream.println was static, so a call through a null PrintStream printed instead of throwing NullPointerException. It is an instance method in Java. - println decoded only named escapes; octal escapes (\0 to \377) printed as written. - createContext() was a valid no-argument call before code was added for the LFSR seed; default it to the empty string.
- println stripped one character from each end of a string literal token, so a
text block printed as "" + its raw text + "". Decode text blocks as JLS 3.10.6
does: drop the delimiters and the line break after the opening one, strip
incidental indentation and trailing white space, then decode escapes,
including a backslash that joins a line to the next.
- The type checker returned early for a class named Object, skipping its
extends clause, so class Object extends Missing {} type checked. Still check
that the clause names a type; the declared Object keeps no parent, since
linking one could leave a cycle the cycle check cannot see.
resOverride searched only the runtime class's own frame, which holds only the methods that class declares. With Base b = new C(), where B overrides f and C extends B, b.f() ran Base.f. Search the runtime class and then its superclasses, nearest first. Predates #79, but every class now inherits Object's natives, so e.g. Object o = new Dog(); o.toString() ran the native instead of an override in Dog's superclass.
The declared Object's extends clause was only looked up, so a direct cycle on the root passed the type checker. javac reports it as cyclic inheritance.
Java 15's space escape is valid in string literals and text blocks, and println printed it as written. In a text block it is decoded after trailing white space is stripped, so it keeps a trailing space.
…ethods resOverride matched candidate descriptors by substring, so a call to f() could run wolf(). It also replaced a private method with a same-signature method in a subclass, though private methods are not overridden. Both predate the superclass walk in 561922b, which made them reachable through every intermediate class.
513c685 to
1913928
Compare
Based on top of #78.
Note to reviewers: This PR appears much, much larger than it actually is because a formatting change was previously applied to the CSE machine files, but was not applied to test files. The test files were formatted in this PR as they were updated here.
Introduction
This feature involves precompiled classes that the CSEC machine uses. This is equivalent to having precompiled library classes offered by a JDK (also commonly referred to as the Java SE API).
Motivation
Library classes and methods implemented in them can be written in Java code. Java programmers have come to naturally expect that common library classes and their methods are readily available. Some of these (in method reference syntax) include:
System.out::printlnObject::hashCodeObject::toStringInstead of injecting these classes as a preamble every time a program is run in the CSE machine, we 'precompile' them, and have them initialised in the environment from the beginning in a single step to avoid extraneous CSE machine states caused by evaluating library classes.
Implementation
Injecting Precompiled Classes
We can 'precompile' Java classes for the CSE machine simply by parsing them and running them as the CSE machine would. The environment that this results in is precisely the initialised environment that should be used for all other programs run in the CSE machine.
We do this before running the main program. The CSEC machine is made to ingest the library classes and thus make them available in the environment before running a new program.
Type Checking Modifications
The type checker must also be able to see such library classes. We achieve this in a similar manner as before, by injecting the type definitions into the type checker. We do so here by just injecting the library code into the type checker before the rest of the program code.
We modify the type checker to ingest any number of programs. It will type check them as though the programs were concatenated one after the other (in order).
We further modify the type checker to use the supplied
Objectclass if provided with one, else it retains its present behaviour.A future PR might look at injecting the definitions directly into the type checker (that is, effectively bypass type checking for library code). This option is more sensible, but may be presently too complicated to be necessary. The intent is to avoid having to do repeated work and have externally imported classes visibly interfere with the running of the CSE machine.
The frontend presently calls the type checker before attempting to run any Java code at all (whether it be through the compiler or the CSE machine). The target of interest is in
frontend/src/commons/utils/JavaHelper.ts. The functionJavaRunhandles calls to the Java CSE machine. Only after the type checker runs successfully will the frontend attempt to run the Java program.This is likely a bad way to do it, because it couples the compiler and CSE machine. A minimal modification has been made to call the CSE machine directly and let the CSE machine handle type checking. This lets us decide what needs to get type-checked. A future PR might look at refactoring this and unifying the different components to use the same library classes.
Note: The library classes implementable in the CSE machine and skeleton classes defined in the JVM (but are presently unimplemented) may genuinely clash. A future PR might look at the feasibility and sensibility of refactoring library classes, and in particular native methods, to be unified across all components. JLS code is no different from normal Java code, so the fact that all of the components presently handle
Objectso specially is concerning (and was a significant source of pain in implementing this feature).As for this particular feature, the type checker can be modified to include the library classes. We isolate the library classes to only the CSE machine.
CSEC Machine Callbacks and Static Lifetimes
In order to implement the target primitives (described below), a number of changes were made to the CSEC Machine's handling of
Contexts. A new field,interfaces, holds all the IO callbacks (stdout,stderr) and new static lifetime items (e.g.LFSR). Correspondingly, various CSEC Machine components have been modified accordingly to propagate this change. Interfaces are presently only used when invoking methods, as only native methods should require and exercise access to items external to the CSEC machine.The need to propagate changes to other components of the CSEC machine uncovers a level of coupling that may be potentially undesirable from a software engineering perspective. It may just be better to pass in the full
Contextto eachCmdEvaluator. (I say potentially because such changes are generally exceedingly rare.)Implemented Primitives
The following primitive functions are supplied as part of this feature.
Object::hashCodeBecause switching states in the CSEC machine always refreshes/reruns the entire program, the hash code generator must be deterministic for a given program, so that hash codes are consistently generated. The easiest way to implement this is to use a PRNG and seed it with the program string (or some constant transformation thereof).
To seed the PRNG with the program string, we first hash it with the FNV1A algorithm (a very simple hashing algorithm). We never have to worry about collisions, because we don't use this hash for anything collision-sensitive at all. This hash is purely just to initialise the PRNG to be used in a deterministic manner.
JS/ECMAScript does not specify that supplied random generators accept seeds. A PRNG is implemented purely for the sole purpose of generating object hash codes. We use an incredibly simple PRNG architecture called a linear feedback shift register (LFSR).
A LFSR is initialised for the
Contextevery time the CSEC machine is run with a program.We skip the mathematical description. The tap selected for the LFSR guarantees that it is of maximal-length. Due to issues with JS' internal representation of numbers, we select a 31-bit maximal-length Fibonacci LFSR with taps at bit 31, 30, 29, and 28.
1 << 31and0, for different reasons.We store the hash code directly on the target that is passed to this method. This is done to mimic JVM implementations. Hash codes are not to be stored as fields on the object instance as the
hashCodemethod is a native method and thus calls a foreign function; this implementation mirrors the recommendation made by the JLS.System.out::printlnThis is an interesting native function to implement. We are forced to properly mimic/mirror the Java class structure used.
System.outis aPrintStreaminstance. In this implementation, we just create a blankPrintStreaminstance and makePrintStream::printlnnative instead.This is purely just so that we can have the same syntax for printing lines. Notably, we do not care about properly building the
Systemclass andPrintStreamobject involved to match that of Java's.Object::toStringWe follow the JLS for this method:
As previously explained, this relies on the behaviour of
hashCode. We do not yet have reflection in the CSE machine, so we implement this as a native method (even though, of course, this could be implemented entirely in Java).