AfflictionNetworks/narya
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

GenStreamableTask flatten: nested classes, region scoping, ancestor-aware supers

Browse Source 
Three structural fixes to flatten mode, found bringing up games on-device
(Epic A1 Phase 5):

1. Nested classes: processFlatten now recurses through getDeclaredClasses()
   (not gated on the outer class's eligibility), so nested Streamables like
   BangConfig.Round get deterministic methods too. All detection/substitution/
   insertion is scoped to each class's own body region (brace-matched,
   comment/string-aware, member-class bodies masked during detection) so an
   instrumented outer class can't hide — or poison — a nested one.

2. Ancestor-aware generation: a class whose ancestor declares read/writeObject
   now generates `super.readObject(ins)` + only the fields declared below that
   ancestor, instead of a self-contained full-set method. A self-contained
   subclass method silently dropped the ancestor's side effects for every
   subclass that previously inherited it: Prop.init() (server NPE cloning
   board props at startRound), Item's _nondb identity extras (itemId=0 on
   every wire-streamed item), InvocationMarshaller's _invdir rebind. Byte
   layout is unchanged: the ancestor's method streams exactly the flattened
   prefix. When the subclass adds no fields, nothing is generated — plain
   inheritance is already complete (the field-free marshaller/unit case).

3. Abstract classes: substitution-only. Their default*Object() pass-throughs
   are replaced in place with the class's own flattened prefix (making them
   safe super-call targets — the reflective pass-through reads the full
   dynamic-class set and would double-read the subclass tail), but whole
   methods are never generated for them. Requires every Streamable ancestor
   to be covered by the task's filesets; see findStreamMethodAncestor doc.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HhfHCXYp2ctWi76Z91y9Ut
This commit is contained in:
Tim Claridge
2026-07-06 21:34:05 +12:00
parent 1cf16721d6
commit 0f1626ecef
1 changed files with 295 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tools/src/main/java/com/threerings/presents/tools/GenStreamableTask.java
+295 -69
View File
@@ -176,17 +176,52 @@ public class GenStreamableTask extends GenTask
* "Flatten" mode: emit self-contained declaration-ordered {@code read/writeObject} that stream
* the whole {@link ClassUtil#getFields} field set via {@link com.threerings.io.GenStreamUtil}.
* Handles {@link DObject}s and replaces pass-through {@code default*Object()} calls in place.
* The top-level class AND every named member class (recursively) are processed — a nested
* Streamable (e.g. {@code BangConfig.Round}) streams reflectively just like a top-level one,
* so it needs the same deterministic methods on ART. All detection/substitution/insertion is
* scoped to each class's own body region so an instrumented outer class never masks (or
* poisons) a nested one and vice versa.
*/
protected void processFlatten (File source, Class<?> sclass)
throws IOException
{
// must be a concrete Streamable; never instrument abstract bases (their concrete subclasses
// stream the full flattened field set themselves, so an abstract base carrying a read/write
// method would "poison" any subclass that didn't get its own — silently dropping fields)
if (!Streamable.class.isAssignableFrom(sclass) || sclass.isInterface() ||
sclass.isEnum() || Modifier.isAbstract(sclass.getModifiers())) {
return;
String src = new String(Files.readAllBytes(source.toPath()));
String out = processFlattenTree(src, sclass);
if (!out.equals(src)) {
writeFile(source.getAbsolutePath(), out);
}
}
/**
* Processes {@code sclass} then recurses into its named member classes. Recursion is NOT gated
* on the enclosing class's eligibility: an abstract (or non-Streamable) outer class can still
* declare concrete nested Streamables that need instrumenting.
*/
protected String processFlattenTree (String src, Class<?> sclass)
{
src = processFlattenClass(src, sclass);
for (Class<?> nested : sclass.getDeclaredClasses()) {
src = processFlattenTree(src, nested);
}
return src;
}
/**
* Instruments a single class (top-level or nested) within {@code src}, returning the updated
* source text (or {@code src} unchanged if the class is ineligible or already instrumented).
*/
protected String processFlattenClass (String src, Class<?> sclass)
{
if (!Streamable.class.isAssignableFrom(sclass) || sclass.isInterface() || sclass.isEnum()) {
return src;
}
// ABSTRACT classes get SUBSTITUTION ONLY: an existing default*Object() pass-through is
// replaced with the class's own flattened prefix (so the method becomes a safe super-call
// target for generated subclass methods — reflective defaultReadObject reads the full
// DYNAMIC-class set, which would double-read the subclass tail), but never whole generated
// methods (those would "poison" any concrete subclass that didn't get its own — silently
// dropping its fields from the stream)
boolean isAbstract = Modifier.isAbstract(sclass.getModifiers());
// gather the streamed fields in the exact order the reflective streamer uses (superclass
// first, then this class's declared fields), skipping @NotStreamable and closure refs
@@ -200,106 +235,248 @@ public class GenStreamableTask extends GenTask
}
fields.add(field);
}
if (fields.isEmpty()) {
return;
// detection and substitution look only at THIS class's body, with any nested member class
// bodies blanked out (space-padded, so offsets in the masked text map 1:1 onto src)
int[] region = findClassRegion(src, sclass);
String masked = maskNestedClasses(src, region, sclass);
boolean hasRead = masked.contains("public void readObject");
boolean hasWrite = masked.contains("public void writeObject");
boolean hasReadDefault = DEFAULT_READ.matcher(masked).find();
boolean hasWriteDefault = DEFAULT_WRITE.matcher(masked).find();
// a field-less class has nothing to generate, but an existing pass-through must STILL be
// substituted (to an empty read/write — the reflective call would stream the full
// dynamic-class set when reached via a subclass's super-call)
if (fields.isEmpty() && !hasReadDefault && !hasWriteDefault) {
return src;
}
String src = new String(Files.readAllBytes(source.toPath()));
boolean hasRead = src.contains("public void readObject");
boolean hasWrite = src.contains("public void writeObject");
boolean hasReadDefault = DEFAULT_READ.matcher(src).find();
boolean hasWriteDefault = DEFAULT_WRITE.matcher(src).find();
// if either method is hand-written custom streaming (present, but NOT a defaultXObject
// pass-through), the class fully manages its own streaming and is already deterministic;
// leave it entirely alone rather than risk a generated/hand-written mismatch
if ((hasRead && !hasReadDefault) || (hasWrite && !hasWriteDefault)) {
return;
return src;
}
// build whole methods for any streaming direction that has no method at all
StringBuilder methods = new StringBuilder();
if (!hasRead) {
methods.append(FLAT_READ_OPEN);
for (Field field : fields) {
methods.append(" ").append(flatReadStatement(field, "ins")).append("\n");
}
methods.append(READ_CLOSE);
// an ancestor that declares its own read/writeObject is the class's streaming "base": a
// generated method here must super-call it (preserving its side effects — Prop.init(),
// InvocationMarshaller's _invdir rebind, Item's _nondb extras) and then stream only the
// fields declared BELOW it. A self-contained full-set method would silently drop those
// side effects for every subclass that previously just inherited the ancestor's method
// (the Phase 5(b) Prop._vscale server NPE / Item._itemId wire bug). Byte layout on the
// stream is unchanged either way: the ancestor's method covers the flattened prefix.
Class<?> readAncestor = findStreamMethodAncestor(
sclass, "readObject", com.threerings.io.ObjectInputStream.class);
Class<?> writeAncestor = findStreamMethodAncestor(
sclass, "writeObject", com.threerings.io.ObjectOutputStream.class);
List<Field> readFields = fieldsBelow(fields, readAncestor);
List<Field> writeFields = fieldsBelow(fields, writeAncestor);
// methods are indented one level deeper than the class declaration's nesting depth
int depth = 0;
for (Class<?> c = sclass; c.getEnclosingClass() != null; c = c.getEnclosingClass()) {
depth++;
}
if (!hasWrite) {
String ind = indent(depth + 1);
// build whole methods for any streaming direction that has no method at all — but when an
// ancestor declares the method and this class adds no fields of its own, plain inheritance
// is already complete and deterministic: generate nothing (the field-free-marshaller case)
StringBuilder methods = new StringBuilder();
if (!hasRead && !isAbstract && !readFields.isEmpty()) {
methods.append(ind).append("// from interface Streamable\n")
.append(ind).append("public void readObject (com.threerings.io.ObjectInputStream ins)\n")
.append(ind).append(" throws java.io.IOException, java.lang.ClassNotFoundException\n")
.append(ind).append("{\n");
if (readAncestor != null) {
methods.append(ind).append(" super.readObject(ins);\n");
}
for (Field field : readFields) {
methods.append(ind).append(" ").append(flatReadStatement(field, "ins")).append("\n");
}
methods.append(ind).append("}\n");
}
if (!hasWrite && !isAbstract && !writeFields.isEmpty()) {
if (methods.length() > 0) {
methods.append("\n");
}
methods.append(FLAT_WRITE_OPEN);
for (Field field : fields) {
methods.append(" ").append(flatWriteStatement(field, "out")).append("\n");
methods.append(ind).append("// from interface Streamable\n")
.append(ind).append("public void writeObject (com.threerings.io.ObjectOutputStream out)\n")
.append(ind).append(" throws java.io.IOException\n")
.append(ind).append("{\n");
if (writeAncestor != null) {
methods.append(ind).append(" super.writeObject(out);\n");
}
methods.append(WRITE_CLOSE);
for (Field field : writeFields) {
methods.append(ind).append(" ").append(flatWriteStatement(field, "out")).append("\n");
}
methods.append(ind).append("}\n");
}
boolean changed = false;
// replace a pass-through default read/write call in place, preserving surrounding logic
if (hasReadDefault) {
src = substituteDefault(src, DEFAULT_READ, fields, true);
// replace pass-through default read/write calls in place, preserving surrounding logic
// (matched against the masked body so a nested class's pass-through is never rewritten
// with THIS class's fields — it gets its own substitution when its class is processed)
if (hasReadDefault || hasWriteDefault) {
src = substituteDefaults(src, masked, region, fields);
changed = true;
}
if (hasWriteDefault) {
src = substituteDefault(src, DEFAULT_WRITE, fields, false);
changed = true;
}
// insert whole methods just before the class-closing brace, so we NEVER disturb the
// insert whole methods just before this class's closing brace, so we NEVER disturb the
// "// AUTO-GENERATED: METHODS" section (which holds the gendobj/genservice accessors) —
// SourceFile.generate() would replace that section wholesale and drop the accessors
if (methods.length() > 0) {
src = insertBeforeClassClose(src, methods.toString());
int closeLineStart = src.lastIndexOf('\n', region[1]) + 1;
src = src.substring(0, closeLineStart) + "\n" + methods + src.substring(closeLineStart);
changed = true;
}
if (changed) {
System.err.println("Converting (flatten) " + sclass.getName() + "...");
writeFile(source.getAbsolutePath(), src);
}
return src;
}
/**
* Inserts {@code methods} (already 4-space indented, newline-terminated) just before the final
* top-level class-closing brace of {@code src}.
* Locates the body of {@code sclass}'s declaration in {@code src}, searching only within the
* enclosing class's own region for nested classes (so same-simple-named classes elsewhere in
* the file can't be confused). Returns {@code {bodyStart, closeBrace}}: the offset just after
* the opening brace and the offset of the matching closing brace.
*/
protected String insertBeforeClassClose (String src, String methods)
protected int[] findClassRegion (String src, Class<?> sclass)
{
String[] lines = src.split("\n", -1);
int lastBrace = -1;
for (int ii = 0; ii < lines.length; ii++) {
if (lines[ii].trim().equals("}")) {
lastBrace = ii;
int searchStart = 0, searchEnd = src.length();
Class<?> outer = sclass.getEnclosingClass();
if (outer != null) {
int[] outerRegion = findClassRegion(src, outer);
searchStart = outerRegion[0];
searchEnd = outerRegion[1];
}
Pattern decl = Pattern.compile(
"\\b(?:class|interface|enum)\\s+" + Pattern.quote(sclass.getSimpleName()) + "\\b");
Matcher m = decl.matcher(src);
m.region(searchStart, searchEnd);
if (!m.find()) {
throw new RuntimeException("Cannot locate declaration of " + sclass.getName());
}
int open = src.indexOf('{', m.end());
if (open < 0 || open >= searchEnd) {
throw new RuntimeException("Cannot locate body of " + sclass.getName());
}
return new int[] { open + 1, matchBrace(src, open) };
}
/**
* Returns the offset of the brace closing the one at {@code open}, skipping braces inside
* string/char literals and comments.
*/
protected int matchBrace (String src, int open)
{
int braces = 0;
for (int ii = open; ii < src.length(); ii++) {
char c = src.charAt(ii);
switch (c) {
case '{':
braces++;
break;
case '}':
if (--braces == 0) {
return ii;
}
break;
case '"':
case '\'':
char quote = c;
for (ii++; ii < src.length(); ii++) {
char d = src.charAt(ii);
if (d == '\\') {
ii++;
} else if (d == quote) {
break;
}
}
break;
case '/':
if (ii + 1 < src.length()) {
char d = src.charAt(ii + 1);
if (d == '/') {
int nl = src.indexOf('\n', ii);
ii = (nl < 0) ? src.length() : nl;
} else if (d == '*') {
int end = src.indexOf("*/", ii + 2);
if (end < 0) {
throw new RuntimeException("Unterminated block comment");
}
ii = end + 1;
}
}
break;
}
}
if (lastBrace < 0) {
throw new RuntimeException("No class-closing brace found");
throw new RuntimeException("Unbalanced braces scanning class body");
}
/**
* Returns {@code src} with everything outside {@code region} AND every named member class body
* within it replaced by spaces — same length as {@code src}, so match offsets in the masked
* text index directly into the original.
*/
protected String maskNestedClasses (String src, int[] region, Class<?> sclass)
{
StringBuilder sb = new StringBuilder(src);
for (int ii = 0; ii < region[0]; ii++) {
blank(sb, ii);
}
StringBuilder sb = new StringBuilder();
for (int ii = 0; ii < lines.length; ii++) {
if (ii == lastBrace) {
sb.append("\n").append(methods);
for (int ii = region[1]; ii < sb.length(); ii++) {
blank(sb, ii);
}
for (Class<?> nested : sclass.getDeclaredClasses()) {
int[] nregion;
try {
nregion = findClassRegion(src, nested);
} catch (RuntimeException e) {
continue; // e.g. synthetic member with no source declaration
}
sb.append(lines[ii]);
if (ii < lines.length - 1) {
sb.append("\n");
for (int ii = nregion[0]; ii < nregion[1]; ii++) {
blank(sb, ii);
}
}
return sb.toString();
}
/**
* Replaces the single {@code recv.defaultReadObject()} / {@code recv.defaultWriteObject()}
* statement matched by {@code pat} with an explicit, same-indentation block that streams every
* field via {@link com.threerings.io.GenStreamUtil}, reusing the receiver variable name.
*/
protected String substituteDefault (String src, Pattern pat, List<Field> fields, boolean read)
protected static void blank (StringBuilder sb, int ii)
{
Matcher m = pat.matcher(src);
StringBuffer sb = new StringBuffer();
while (m.find()) {
if (sb.charAt(ii) != '\n') {
sb.setCharAt(ii, ' ');
}
}
/**
* Replaces every pass-through {@code recv.defaultReadObject()} / {@code recv.defaultWriteObject()}
* statement found in {@code masked} with an explicit, same-indentation block that streams every
* field via {@link com.threerings.io.GenStreamUtil}, reusing the receiver variable name.
* Matching runs on the masked text (this class's body only); edits apply to the real source.
*/
protected String substituteDefaults (String src, String masked, int[] region, List<Field> fields)
{
// collect matches from both patterns, then apply in reverse offset order so earlier
// replacements don't shift later match positions
List<int[]> matches = Lists.newArrayList(); // {start, end, isRead}
for (Pattern pat : new Pattern[] { DEFAULT_READ, DEFAULT_WRITE }) {
Matcher m = pat.matcher(masked);
while (m.find()) {
matches.add(new int[] { m.start(), m.end(), (pat == DEFAULT_READ) ? 1 : 0 });
}
}
matches.sort((a, b) -> b[0] - a[0]);
StringBuilder sb = new StringBuilder(src);
for (int[] match : matches) {
Matcher m = ((match[2] == 1) ? DEFAULT_READ : DEFAULT_WRITE).matcher(masked);
m.region(match[0], match[1]);
if (!m.find()) {
throw new RuntimeException("Lost a default*Object match on re-find");
}
String indent = m.group(1), recv = m.group(2);
StringBuilder block = new StringBuilder();
for (int ii = 0; ii < fields.size(); ii++) {
@@ -307,15 +484,64 @@ public class GenStreamableTask extends GenTask
block.append("\n");
}
Field field = fields.get(ii);
block.append(indent).append(
read ? flatReadStatement(field, recv) : flatWriteStatement(field, recv));
block.append(indent).append((match[2] == 1)
? flatReadStatement(field, recv) : flatWriteStatement(field, recv));
}
m.appendReplacement(sb, Matcher.quoteReplacement(block.toString()));
sb.replace(match[0], match[1], block.toString());
}
m.appendTail(sb);
return sb.toString();
}
protected static String indent (int levels)
{
StringBuilder sb = new StringBuilder();
for (int ii = 0; ii < levels; ii++) {
sb.append(" ");
}
return sb.toString();
}
/**
* Returns the nearest strict ancestor of {@code sclass} that declares the given streaming
* method, or null. Such an ancestor is a safe super-call target because flatten mode
* guarantees its method streams exactly the ancestor's own flattened prefix: hand-written
* methods with a {@code default*Object()} pass-through get it substituted in place (abstract
* classes included — see {@link #processFlattenClass}), and generated methods are explicit by
* construction. REQUIREMENT: every Streamable ancestor's source must be covered by this
* task's filesets (or already be prefix-exact, like narya's own InvocationMarshaller) — an
* out-of-fileset ancestor whose method still calls the reflective pass-through would read the
* full dynamic-class field set and double-read the subclass tail.
*/
protected Class<?> findStreamMethodAncestor (Class<?> sclass, String name, Class<?> arg)
{
for (Class<?> c = sclass.getSuperclass(); c != null; c = c.getSuperclass()) {
try {
c.getDeclaredMethod(name, arg);
return c;
} catch (NoSuchMethodException nsme) {
// keep walking
}
}
return null;
}
/** Returns the subset of {@code fields} declared strictly below {@code ancestor} (all of
* them when {@code ancestor} is null). */
protected static List<Field> fieldsBelow (List<Field> fields, Class<?> ancestor)
{
if (ancestor == null) {
return fields;
}
List<Field> below = Lists.newArrayList();
for (Field field : fields) {
Class<?> declaring = field.getDeclaringClass();
if (declaring != ancestor && ancestor.isAssignableFrom(declaring)) {
below.add(field);
}
}
return below;
}
protected String flatReadStatement (Field field, String recv)
{
return "com.threerings.io.GenStreamUtil.readField(" +