Go: model for ... range as a rich Go.RangeLoop instead of cramming it into J.ForEachLoop#7843
Open
knutwannheden wants to merge 1 commit into
Open
Go: model for ... range as a rich Go.RangeLoop instead of cramming it into J.ForEachLoop#7843knutwannheden wants to merge 1 commit into
for ... range as a rich Go.RangeLoop instead of cramming it into J.ForEachLoop#7843knutwannheden wants to merge 1 commit into
Conversation
…it into J.ForEachLoop Go's range loop binds a key and an optional value joined by `:=`/`=`, which cannot fit Java's single-variable J.ForEachLoop.Control. The Go side had defined its own fatter `java.ForEachControl` (Key/Value/Operator/Iterable) masquerading under the Java type name and bridged it to the real J.ForEachLoop.Control via a custom RPC codec — but that codec truncated to a single synthesized variable, dropping the value and the `:=`/`=` distinction. Under a recipe (full deserialization) `for k, v := range x` came back as `for_:=range x`. Introduce a dedicated Go.RangeLoop node (key?, value?, operator, iterable, body) on both sides, mirroring the per-language loop-node pattern (JS ForOfLoop/ForInLoop). Remove the lossy ForEachLoop/ForEachControl cheat entirely: the Go-customized java.ForEachLoop/ForEachControl structs, their RPC send/receive, the GolangSender/GolangReceiver bridge overrides, and the J$ForEachLoop wire types are all gone. Adds range round-trip coverage (key+value, key-only, no-variable, `=` assign) to GoRpcPrintRoundTripIntegTest.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Running a recipe over Go source mangled every
for ... rangeloop:Go's range loop binds a key and an optional value joined by
:=or=(for k, v := range x,for k := range x,for range x). That doesn't fit Java's single-variableJ.ForEachLoop.Control(onevariable+iterable). The Go side worked around it by defining its own fatterjava.ForEachControl(Key/Value/Operator/Iterable) masquerading under the Java type name, then bridging it to the realJ.ForEachLoop.Controlover RPC with a hand-written codec.J.ForEachLoop.Controlhas nowhere to put a second variable or the:=/=, so the codec synthesized a single variable from the key and dropped the value and the operator. Like the operator fixes in Go: map Go-specific operators to rich Go.* LST nodes so they survive RPC round-trips #7842, it round-trips fine Go↔Go (both sides use the fat struct) but truncates on the Java leg — invisible until a recipe forces full deserialization.Examples
New node (mirrors the per-language loop-node pattern — JS has
ForOfLoop/ForInLoop):Summary
Go.RangeLoopend-to-end (JavaGo.java/GolangVisitor/GolangSender/GolangReceiver; Go tree + parser + printer + visitor + RPC sender/receiver + factory &RegisterValueType).java.ForEachLoop/java.ForEachControlstructs and methods, their RPC send/receive, the lossyvisitForEachControl/visitForEachLoopbridge overrides, and theJ$ForEachLoop/J$ForEachLoop$Controlwire registrations. Template matching (comparator/substitution/go_template) now matchesGo.RangeLoop.AssignOp(Equals/Define) is kept and reused as the operator enum, round-tripping as theGo.RangeLoop.Typeconstant name.Note for reviewers: a Go LST node needs both
RegisterFactory(receive) andRegisterValueType(send) — registering only one silently desyncs the wire into aParseError.java.*struct carries more than its realJ.*counterpart and truncates over RPC (follow-up to Go: map Go-specific operators to rich Go.* LST nodes so they survive RPC round-trips #7842). Known remaining, each its own follow-up PR: multi-valuereturn,if init; cond,switch init; tag, groupedvar(...)/const(...)blocks.Test plan
GoRpcPrintRoundTripIntegTest(key+value, key-only, no-variable,=assign) — all green./gradlew :rewrite-go:test :rewrite-go:integTestpasses (no regressions)go test ./...passes