cb.NewClosureWith support optional auto-lambda category

[xushiwei]

• Proposal: Auto Lambda Control Flow Semantics xgo#2834

[codecov]

Codecov Report

❌ Patch coverage is 88.23529% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.69%. Comparing base (f236882) to head (a34bbe6).

Files with missing lines	Patch %	Lines
codebuild.go	80.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #642      +/-   ##
==========================================
- Coverage   93.69%   93.69%   -0.01%     
==========================================
  Files          29       29              
  Lines        7237     7243       +6     
==========================================
+ Hits         6781     6786       +5     
- Misses        388      389       +1     
  Partials       68       68              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[fennoai]

Review: cb.NewClosureWith support optional auto-lambda category

Nice, minimal design: threading flows out of endFuncBody reuses the existing codeBlockCtx.flows machinery instead of inventing a parallel mechanism, the variadic parameter keeps NewClosureWith source-compatible, and renaming cantUseFlows -> cantUseFlowsInForRange is good hygiene alongside the new constant.

go build ./..., go vet ./... and go test ./... all pass on a34bbe6.

I verified each finding below by writing throwaway tests against the PR head rather than reasoning from the diff alone. Two things I want to flag about that:

Verified real (see inline comments):

• cate != nil panics on an empty non-nil slice — reproduced index out of range [0] with length 0.
• A goto to a label defined inside the same auto lambda is rejected. Reproduced end-to-end: can't use return/continue/break/goto in auto lambda for a fully self-contained local goto.
• After the error, generation continues and emits uncompilable Go. With an accumulating HandleErr, a lambda containing break produced:

  func main() {
      func() {
          break
      }
  }

Checked and found NOT to be a problem — worth recording so it doesn't get "fixed" unnecessarily: a self-contained labeled break (L: for { break L }) inside an auto lambda is not falsely rejected. Although flowFlagWithLabel is never explicitly masked, I instrumented cb.current.flows after the labeled for and it is 0, so the labeled case is already handled. Only the goto path leaks.

Design questions (no inline location)

• AutoLambdaCond and AutoLambdaLoop are behaviorally identical. The only read of p.cate is p.cate != AutoLambdaNormal (func.go:106), so the enum currently encodes a boolean. Is the distinction reserved for future use, or should the two reject different flow kinds? Concretely: for a cond lambda, should return really be illegal? Note ConvertToClosure builds its return via emitReturnStmt, which does not set flowFlagReturn, so today the return bit only fires for explicit CodeBuilder.Return calls. Differentiating (e.g. allow return for Cond, reject for Loop) would give the two constants an actual reason to be distinct.
• Error position points at the whole lambda, not the offending statement. getSrcPos(src)/getSrcEnd(src) is the closure's own node, so a multi-line lambda reports its start rather than the return/break that is actually illegal — and with src == nil the error renders position-less (my repro showed -: as the position). Since these errors surface in XGo compiler output, consider recording the position of the first flow statement alongside the flags.
• Docs for a public API. The three exported constants have no doc comments, and "auto-lambda" is defined nowhere in the repo (searched: it appears only in codebuild.go, func.go, error_msg_test.go). NewClosureWith's comment also doesn't state that omitting the argument means AutoLambdaNormal, nor that passing a category makes return/continue/break/goto an error at End(). Minor: docs say "auto-lambda", the error string says "auto lambda".
• Test coverage. TestErrAutoLambda covers exactly one combination (AutoLambdaLoop + Return). The error names four flow kinds; AutoLambdaCond and the negative case (AutoLambdaNormal with flows present must not error) are untested.
• Naming nit. Consider naming endFuncBody's new result (stmts []target.Stmt, flows int) — the bare int is only interpretable by finding flowFlag*.

[fennoai]

Verified panic: use len(cate) > 0 instead of cate != nil.

cate != nil is not equivalent to len(cate) > 0. A caller forwarding its own optional slice — NewClosureWith(sig, cates...) where cates is non-nil but empty — passes a non-nil zero-length slice, and cate[0] panics. I confirmed this against the PR head:

PANIC: runtime error: index out of range [0] with length 0

for cates := []gogen.AutoLambdaCategory{}; cb.NewClosureWith(sig, cates...). This also contradicts the "optional" wording in the doc comment.

c := AutoLambdaNormal
if len(cate) > 0 {
	c = cate[0]
}

[fennoai]

flows != 0 falsely rejects a goto whose label is defined inside the same lambda.

flowFlagGoto is set unconditionally by Goto (codebuild.go:2243) and never cleared, so a jump that is entirely local to the lambda body — valid Go — trips this check. Reproduced end-to-end on this head:

cb.NewClosureWith(sig, gogen.AutoLambdaLoop).BodyStart(pkg)
l := cb.NewLabel(token.NoPos, token.NoPos, "G")
cb.Goto(l)
cb.Label(l).None().EndStmt()
cb.End(nil)
// => can't use return/continue/break/goto in auto lambda

Labels are per-funcBodyCtx (reset in startFuncBody), so "target defined in this closure" is cheaply determinable. Two options:

• Precise: only set flowFlagGoto when the target label is not defined in the current funcBodyCtx.
• Cheap: check flows & (flowFlagBreak|flowFlagContinue|flowFlagReturn) != 0, which is strictly fewer false positives than today.

For the record, I did check the labeled-break case (L: for { break L }) and it is not affected — flows is 0 there despite flowFlagWithLabel never being masked. Only goto leaks.

[fennoai]

After this error, generation continues and emits uncompilable Go.

handleCodeError routes to Config.HandleErr and falls through, so End still pushes a FuncLit whose body contains the illegal break/continue/goto. With an accumulating handler (the XGo compiler's mode), I got both the diagnostic and this output:

package main

func main() {
	func() {
		break
	}
}

which does not compile. The structurally identical for-range check uses panicCodeError (util_gengo.go:1182), so the two illegal-flow paths now behave differently.

Accumulating is arguably the better choice — it lets the compiler collect multiple diagnostics — but then the offending body should be neutralized (drop the flow statements, or emit an empty body) so gogen never emits invalid source. Otherwise use panicCodeError here for consistency with the for-range site.

[fennoai]

Discarding flows here may be a real gap, not just a mechanical _.

Unlike a real closure, an inline closure's body is spliced into the enclosing block, so control flow inside it belongs to the enclosing block's flow set. ReturnErr(outer: true) emits a genuine return and sets flowFlagReturn on this context (codebuild.go:433) — dropped here. If such an inline closure sits inside an auto lambda or a for range udt.XGo_Enum(callback), the escaping return won't trip either check.

Note you can't blindly propagate everything: Return inside an inline closure lowers to Goto(endingLabel) with the label emitted in the same inlined block (codebuild.go:480), so that flowFlagGoto is genuinely self-contained. Something like cb.current.flows |= flows &^ flowFlagGoto — or local-label tracking as above — is probably right.

If the discard is deliberate, a short comment instead of a bare _ would help; as written it reads like fallout from the signature change.

[fennoai]

Exported constants have no doc comments. These are part of a public API consumed by the XGo compiler and will render on pkg.go.dev as a bare const block. Nearby code in this repo does document constants inline (e.g. FlagDepModXGo = 1 << iota // depends module github.com/goplus/xgo in package.go).

Worth stating for each value what it means and which restriction it implies — especially since Cond and Loop are currently indistinguishable in behavior, so a caller has no way to tell which to pass.