Add foundational support for class/record/sealed types - #440
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds initial support for Java class/record reference types in the Java→Laurel translation by treating them as opaque, uninterpreted composite sorts, and introduces minimal handling for new and instanceof expressions.
Changes:
- Translate
Type.ClassTypeto an opaqueCompositeTypeusing the Java type name. - Convert
new T(...)into an opaque Laurelnew_(T)value (discarding constructor args after translation). - Convert
x instanceof Tinto a call to aninstanceOf_<T>(x)predicate.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4bf6ab4 to
d9f38b0
Compare
fabiomadge
left a comment
There was a problem hiding this comment.
Locally (origin/main, same Strata e115b8ec): a class/record-typed parameter doesn't verify — static void acceptParam(Point p){} → Error: Resolution failed: '...Point' is not defined. The opaque CompositeType is referenced but never declared (no compositeCommand/prelude entry), so Strata's resolver can't find the sort — looks like it needs an actual type declaration emitted.
(1) Is param-position acceptance meant to verify yet, or just translate? (2) No test in the PR, so CI doesn't catch it — one class-typed method would. (Same shape as my #441 note.)
d9f38b0 to
12f22f6
Compare
|
@fabiomadge I believe I have addressed your comments with 12f22f6. |
fabiomadge
left a comment
There was a problem hiding this comment.
Update on 12f22f6: the declaration fix works for types in type position (params/locals/returns verify, cross-unit too). But the new path has a gap — a type used only in new T(...), never in a type position, isn't registered, so it still hits the original error:
record Q(int x){}
static void make() { Object o = new Q(1); }
// Resolution failed: '...Q' is not definedtranslateType does referencedCompositeTypes.add(sortName); the JCNewClass case forms the same name and calls new_(name) without registering it. One-line fix + a new-only test.
Minor: composite sorts use the FQ dotted name, but qualifiedMethodName uses packageless Outer_name — inconsistent (the FQ scheme is the safer one).
Soundness etc. look good (opaque impure composite = Java reference ==; distinct fresh objects not provably equal; instanceof rejects cleanly). Just the new-path gap.
…parameter position Encode a Java class/interface type (including sealed hierarchies and records) as an opaque Laurel CompositeType named after the type's fully-qualified name (with `$` normalised to `.`). Keeping the package makes the sort name stable and collision-free. Crucially, each referenced composite sort is now *declared* — an empty compositeCommand emitted (once, before the procedures that use it) so Strata's resolver can find the sort. Without the declaration a class/record-typed parameter failed with "Resolution failed: '...' is not defined" rather than verifying. Adds AcceptClassParam, a test with a record-typed parameter method that verifies cleanly. Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
…d types Translate `new T(...)` to an opaque Laurel `new_(T)` value of the matching CompositeType. Constructor arguments are translated (so unsupported argument expressions still surface as errors) but otherwise discarded: the opaque value models reference identity only, not the constructor's arguments or their side effects. The composite sort is registered for declaration here too, so a type that appears only in `new T(...)` (and never in a type position such as a parameter, local, or return) still gets its sort declared and the `new_(T)` value resolves. Adds NewOnlyRecordType, a test for a record used solely in `new` position. Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
…h a clear error In the opaque-CompositeType encoding a class/record value carries no runtime tag, so `r instanceof X` cannot be modelled precisely. Fail at translation with a targeted JavaViolationException attributable to the `instanceof` feature, instead of emitting an undeclared `instanceOf_<X>` predicate symbol — which surfaced only as a confusing downstream "Resolution failed" message and, using the simple type name, could even collide across packages. A precise encoding needs a tagged datatype representation (future work). Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
qualifiedMethodName used the outermost class's packageless simple name (Outer_method), while composite sorts use the fully-qualified dotted name. Two same-named classes in different packages could therefore produce colliding procedure names. Align procedure names with the composite-sort scheme: use the outermost class's fully-qualified name (with '$' normalised to '.'). The name is used for both the procedure declaration and its call sites (via the same helper), so they stay matched. Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
12f22f6 to
d4abe13
Compare
|
@fabiomadge I believe all your comments have been addressed. |
fabiomadge
left a comment
There was a problem hiding this comment.
Both my findings are fixed and verified: the composite sort is now declared, and the new-only-type gap is closed (referencedCompositeTypes.add(name) in the JCNewClass case) with a NewOnlyRecordType regression test — my repro verifies now. The qualifiedMethodName FQ-rename (addressing the naming-consistency note) passes the full verifier suite with no regressions.
Two non-blocking notes for the record: (1) records are modeled as opaque references — sound for identity/==, but component/value reasoning is deferred (documented); (2) the FQ method-name change is broad — I validated it via the full suite, but didn't exhaustively probe overloaded-name collisions (the existing TODO in that method still applies). Sound foundation. LGTM.
What was changed?
Adds foundational support for class / record / sealed types to the Java → Laurel front end (
JavaToLaurelCompiler). Such types are modelled as opaque LaurelCompositeTypes — Strata treats an unbound composite type as an uninterpreted reference sort — which is enough to accept these values and reason about identity/reference-style properties.Commits:
CompositeTypenamed after its fully-qualified name ($→., package kept for stability/uniqueness). Each referenced sort is declared (an emptycompositeCommand, emitted once before the procedures that use it) so Strata's resolver can find it. Test:AcceptClassParam.new T(...)construction — translate to an opaquenew_(T)value. The sort is registered here too, so a type appearing only innew T(...)(never in a type position) is still declared and resolves. Test:NewOnlyRecordType.instanceof— explicitly rejected with a clearJavaViolationException(the opaque encoding has no runtime tag).qualifiedMethodNameused the outermost class's packageless simple name (Outer_method), inconsistent with the FQ composite-sort scheme; two same-named classes in different packages could collide. Aligned to the FQ scheme (the helper names both declarations and call sites, so they stay matched).How has this been tested?
./gradlew :verifier:testpasses, includingAcceptClassParam(type-position acceptance) andNewOnlyRecordType(a record used solely innewposition).Modelling notes / known limitations
Opaque, reference-level foundation — enough for identity/aliasing-style reasoning, not yet a structural encoding:
instanceofis not supported and fails fast at translation.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0.