Skip to content

Func.End: support AutoLambdaCond/Loop - #643

Merged
xushiwei merged 3 commits into
goplus:mainfrom
xushiwei:q
Aug 14, 2026
Merged

Func.End: support AutoLambdaCond/Loop#643
xushiwei merged 3 commits into
goplus:mainfrom
xushiwei:q

Conversation

@xushiwei

@xushiwei xushiwei commented Aug 14, 2026

Copy link
Copy Markdown
Member

@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.67%. Comparing base (a34bbe6) to head (bce1b2b).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
util_genjs.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #643      +/-   ##
==========================================
- Coverage   93.69%   93.67%   -0.02%     
==========================================
  Files          29       29              
  Lines        7243     7262      +19     
==========================================
+ Hits         6786     6803      +17     
- Misses        389      391       +2     
  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.

@xushiwei
xushiwei merged commit f19345f into goplus:main Aug 14, 2026
18 of 20 checks passed

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: auto-lambda flow-control rework

The change is well-structured. Reporting return misuse early via panicCodeErrorf gives a precise position (as TestErrAutoLambda now shows), gating the "missing return" check to AutoLambdaNormal is correct, and the genjs stub keeps the build-tag-split files in sync. Performance and security passes found nothing of concern (the p.current.fn nil-deref in Return() pre-dates this PR and mirrors ReturnErr(); the util_genjs.go stub is behind //go:build genjs and unreachable in the default build).

One correctness question worth resolving (inline) plus a couple of minor notes below.

Minor — error-reporting asymmetry. Return() now reports the auto-lambda violation immediately via panicCodeErrorf at the offending statement, while break/continue/goto are still detected only at Func.End() via flows != 0, reported through handleCodeError at the lambda's closing position. So the two differ in both position precision (5:2 for return vs 6:1 for break in the test) and delivery (immediate panic vs the overridable/accumulating handler). If a caller runs the handler in error-collection mode, return will unwind while the branch statements accumulate. Consider a brief comment explaining why return is special-cased, or giving the branch statements the same early treatment for consistency.

Comment thread util_gengo.go
}
}

func return0IfNeeded(stmts []ast.Stmt) []ast.Stmt {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] return0IfNeeded hardcodes return 0, invalid for non-int result types

return0IfNeeded unconditionally appends return 0 (a token.INT BasicLit "0") when the body doesn't already end in a ReturnStmt, ignoring the closure signature entirely. This only produces valid Go when the auto-lambda has a single numeric result:

  • Non-numeric result (string, bool, struct, pointer) → func() string { return 0 } is invalid.
  • Zero results → return 0 is a too many return values error.
  • Multiple results → not enough return values.

The tests (TestAutoLambda1/2) only cover a single int result, so this gap is untested. If callers guarantee auto-lambdas always have a single integer result, please document/assert that invariant; otherwise derive the zero-value expression from the signature's result type instead of hardcoding "0".

Comment thread codebuild.go
}

// For func
// For loop statement: For (..|None) Then [.. Post] .. End

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] For doc comment ordering reads reversed

The new comment // For loop statement: For (..|None) Then [.. Post] .. End groups [.. Post] as the optional element with the bare body .. after it, which reads as "optional post-section, then body" — the reverse of the actual builder order. TestFor and the stmt.go grammar block show: For → init/cond (or None) → Thenbody → optional (Post → post-stmt) → End. Suggest e.g. For (..|None) Then .. [Post ..] End.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant