Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 22 additions & 9 deletions codebuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,13 +287,12 @@ func insertParams(scope *types.Scope, params *types.Tuple) {
}
}

func (p *CodeBuilder) endFuncBody(old funcBodyCtx) []target.Stmt {
func (p *CodeBuilder) endFuncBody(old funcBodyCtx) ([]target.Stmt, int) {
p.current.checkLabels(p)
p.current.fn = old.fn
p.current.labels = old.labels
p.current.panicCalls = old.panicCalls
stmts, _ := p.endBlockStmt(&old.codeBlockCtx)
return stmts
return p.endBlockStmt(&old.codeBlockCtx)
}

func (p *CodeBuilder) startBlockStmt(current codeBlock, src []ast.Node, comment string, old *codeBlockCtx) *CodeBuilder {
Expand Down Expand Up @@ -587,7 +586,8 @@ func (p *Func) inlineClosureEnd(cb *CodeBuilder) {
cb.Label(ending)
}
sig := p.Type().(*types.Signature)
cb.emitStmt(&target.BlockStmt{List: cb.endFuncBody(p.old)})
fnBody, _ := cb.endFuncBody(p.old)

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.

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.

cb.emitStmt(&target.BlockStmt{List: fnBody})
cb.stk.PopN(p.getInlineCallArity())
results := sig.Results()
for i, n := 0, results.Len(); i < n; i++ { // return results & clean env
Expand Down Expand Up @@ -642,15 +642,28 @@ func (p *CodeBuilder) emitVar(pkg *Package, closure *Func, param *types.Var, wit
p.paramInsts[key] = p.current.scope.Lookup(name).(*types.Var)
}

// NewClosure func
// NewClosure creates a new closure.
func (p *CodeBuilder) NewClosure(params, results *types.Tuple, variadic bool) *Func {
sig := types.NewSignatureType(nil, nil, nil, params, results, variadic)
return p.NewClosureWith(sig)
return p.NewClosureWith(sig, AutoLambdaNormal)
}

// NewClosureWith func
func (p *CodeBuilder) NewClosureWith(sig *types.Signature) *Func {
return p.pkg.newClosure(sig)
// AutoLambdaCategory represents the category of an auto-lambda.
type AutoLambdaCategory int

const (

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.

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.

AutoLambdaNormal AutoLambdaCategory = iota
AutoLambdaCond
AutoLambdaLoop
)

// NewClosureWith creates a new closure with an optional auto-lambda category.
func (p *CodeBuilder) NewClosureWith(sig *types.Signature, cate ...AutoLambdaCategory) *Func {
var c AutoLambdaCategory
if cate != nil {

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.

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]
}

c = cate[0]
}
return p.pkg.newClosure(sig, c)
}

// ConvertToClosure converts an expression into a closure.
Expand Down
11 changes: 11 additions & 0 deletions error_msg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,17 @@ func TestErrSwitch(t *testing.T) {
})
}

func TestErrAutoLambda(t *testing.T) {
codeErrorTest(t, `./foo.gop:5:1: can't use return/continue/break/goto in auto lambda`, func(pkg *gogen.Package) {
cb := pkg.NewFunc(nil, "main", nil, nil, false).BodyStart(pkg)
sig := types.NewSignatureType(nil, nil, nil, nil, nil, false)
cb.NewClosureWith(sig, gogen.AutoLambdaLoop).BodyStart(pkg).
Return(0).
End(source("foo", 5, 1))
cb.End()
})
}

func TestErrTypeRedefined(t *testing.T) {
codeErrorTest(t, "./foo.gop:2:5: foo redeclared in this block\n\tprevious declaration at ./foo.gop:1:5", func(pkg *gogen.Package) {
typ := pkg.NewType("foo", source("foo", 1, 5))
Expand Down
15 changes: 12 additions & 3 deletions func.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ func (p *Package) NewParam(pos token.Pos, name string, typ types.Type, optional
type Func struct {
*types.Func
decl *funcDecl
cate AutoLambdaCategory
old funcBodyCtx
arity1 int // 0 for normal, (arity+1) for inlineClosure
}
Expand Down Expand Up @@ -89,6 +90,10 @@ func (p *Func) BodyStart(pkg *Package, src ...ast.Node) *CodeBuilder {
return pkg.cb.startFuncBody(p, src, &p.old)
}

const (
cantUseFlowsInAutoLambda = "can't use return/continue/break/goto in auto lambda"
)

// End is for internal use.
func (p *Func) End(cb *CodeBuilder, src ast.Node) {
if p.isInline() {
Expand All @@ -97,7 +102,11 @@ func (p *Func) End(cb *CodeBuilder, src ast.Node) {
}
pkg := cb.pkg
checker := termChecker{cb.current.panicCalls}
body := &target.BlockStmt{List: cb.endFuncBody(p.old)}
fnBody, flows := cb.endFuncBody(p.old)
if flows != 0 && p.cate != AutoLambdaNormal {

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.

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.

cb.handleCodeError(getSrcPos(src), getSrcEnd(src), cantUseFlowsInAutoLambda)

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.

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.

}
body := &target.BlockStmt{List: fnBody}
t := p.Type().(*types.Signature)

// Check for missing return at the closing brace position.
Expand Down Expand Up @@ -252,9 +261,9 @@ func (p *Package) NewFuncWith(
return fn, nil
}

func (p *Package) newClosure(sig *types.Signature) *Func {
func (p *Package) newClosure(sig *types.Signature, cate AutoLambdaCategory) *Func {
fn := types.NewFunc(token.NoPos, p.Types, "", sig)
return &Func{Func: fn}
return &Func{Func: fn, cate: cate}
}

func (p *Package) newInlineClosure(sig *types.Signature, arity int) *Func {
Expand Down
4 changes: 2 additions & 2 deletions util_gengo.go
Original file line number Diff line number Diff line change
Expand Up @@ -1179,7 +1179,7 @@ func emitForRangeStmt(cb *CodeBuilder, p *forRangeStmt, stmts []ast.Stmt, flows
})
*/
if flows != 0 {
cb.panicCodeError(p.stmt.For, p.stmt.For, cantUseFlows)
cb.panicCodeError(p.stmt.For, p.stmt.For, cantUseFlowsInForRange)
}
n = -n
def := p.stmt.Tok == token.DEFINE
Expand Down Expand Up @@ -1214,7 +1214,7 @@ func emitForRangeStmt(cb *CodeBuilder, p *forRangeStmt, stmts []ast.Stmt, flows
}

const (
cantUseFlows = "can't use return/continue/break/goto in for range of udt.XGo_Enum(callback)"
cantUseFlowsInForRange = "can't use return/continue/break/goto in for range of udt.XGo_Enum(callback)"
)

var (
Expand Down