Skip to content

cff panics when cff.Invoke argument's value cannot be inferred #103

Description

@ZhangZhuoSJTU

Hi! Thanks a lot for building and maintaining this very cool project. It's been great to explore!

I ran into a crash that I think could be handled more gracefully. Consider the following minimal repro:

//go:build cff

package example

import (
    "context"
    "go.uber.org/cff"
)

func MyTask(ctx context.Context, input string) (string, error) {
    return "Hello, " + input, nil
}

func RunFlow(ctx context.Context, input string) (string, error) {
    var result string
    var a bool = true
    err := cff.Flow(
        ctx,
        cff.Task(MyTask, cff.Invoke(a)),
    )
    return result, err
}

In this example, the argument to cff.Invoke is a bool variable a, which has a constant value true. Note that a itself is a variable (not a const).

With our go/packages configuration:

const _goPackagesLoadMode = packages.NeedName |
packages.NeedFiles |
packages.NeedCompiledGoFiles |
packages.NeedImports |
packages.NeedDeps |
packages.NeedTypes |
packages.NeedSyntax |
packages.NeedTypesInfo |
packages.NeedTypesSizes

we don't get constant propagation/evaluation, so the parsed Value of this a ends up being <nil> even though its type is bool (and actually, I guess packages.Load cannot support constant propagation or evaluation by its nature?).

As a result, when the compiler tries to compileInvoke:

cff/internal/compile.go

Lines 994 to 1008 in 4cdfe95

func (c *compiler) compileInvoke(flow *flow, o *ast.CallExpr) *noOutput {
// Bool type checking is satisfied by cff.Invoke interface.
if len(o.Args) != 1 {
c.errf(c.nodePosition(o.Fun), "invoke expects exactly one argument")
}
val, ok := c.info.Types[o.Args[0]]
if !ok {
c.errf(c.nodePosition(o), "expected to find a bool, found %v instead", astutil.NodeDescription(o.Args[0]))
return nil
}
if constant.BoolVal(val.Value) {
return flow.addNoOutput()
}
return nil
}

The value of val and ok in val, ok := c.info.Types[o.Args[0]] will be {Type: bool, Value: <nil>} and true, respectively.

After that, when the code goes into constant.BoolVal(val.Value), whose implementation is

func BoolVal(x Value) bool {
	fmt.Println("here!", x)
	switch x := x.(type) {
	case boolVal:
		return bool(x)
	case unknownVal:
		return false
	default:
		panic(fmt.Sprintf("%v not a Bool", x))
	}
}

A panic will be triggered.

panic: <nil> not a Bool

goroutine 1 [running]:
go/constant.BoolVal({0x0, 0x0})
	/opt/homebrew/Cellar/go/1.22.1/libexec/src/go/constant/value.go:488 +0x138
go.uber.org/cff/internal.(*compiler).compileInvoke(0x14001e89100, 0x1400743a840, 0x14001e887c0)
	/Users/izhuer/Code/cff/internal/compile.go:1005 +0x1cc
go.uber.org/cff/internal.(*compiler).interpretTaskOptions(0x14001e89100, 0x1400743a840, 0x14005aca870, {0x140029f0cf0?, 0x140001587e8?, 0x102e3b9f0?})
	/Users/izhuer/Code/cff/internal/compile.go:868 +0xfc
go.uber.org/cff/internal.(*compiler).compileTask(0x14001e89100, 0x1400743a840, {0x1031b8588, 0x140029f0c60}, {0x140029f0cf0, 0x1, 0x1})
	/Users/izhuer/Code/cff/internal/compile.go:670 +0x264
go.uber.org/cff/internal.(*compiler).compileFlow(0x14001e89100, 0x140021165a0?, 0x14001e88840)
	/Users/izhuer/Code/cff/internal/compile.go:400 +0x47c
go.uber.org/cff/internal.(*compiler).compileFile.func1({0x1031b7610?, 0x14001e88840})
	/Users/izhuer/Code/cff/internal/compile.go:157 +0x3c8
go.uber.org/cff/internal.visitorFunc.Visit(0x140029f0e60, {0x1031b7610?, 0x14001e88840?})
	/Users/izhuer/Code/cff/internal/ast.go:8 +0x38
go/ast.Walk({0x1031b6968?, 0x140029f0e60?}, {0x1031b7610, 0x14001e88840})
	/opt/homebrew/Cellar/go/1.22.1/libexec/src/go/ast/walk.go:51 +0x44
go/ast.walkExprList(...)
	/opt/homebrew/Cellar/go/1.22.1/libexec/src/go/ast/walk.go:26
go/ast.Walk({0x1031b6968?, 0x140029f0e60?}, {0x1031b7dd8, 0x14001e888c0})
	/opt/homebrew/Cellar/go/1.22.1/libexec/src/go/ast/walk.go:217 +0x2900
go/ast.walkStmtList(...)
	/opt/homebrew/Cellar/go/1.22.1/libexec/src/go/ast/walk.go:32
go/ast.Walk({0x1031b6968?, 0x140029f0e60?}, {0x1031b7a18, 0x140021162d0})
	/opt/homebrew/Cellar/go/1.22.1/libexec/src/go/ast/walk.go:234 +0x29f4
go/ast.Walk({0x1031b6968?, 0x140029f0e60?}, {0x1031b8080, 0x14002116360})
	/opt/homebrew/Cellar/go/1.22.1/libexec/src/go/ast/walk.go:357 +0xd90
go/ast.walkDeclList(...)
	/opt/homebrew/Cellar/go/1.22.1/libexec/src/go/ast/walk.go:38
go/ast.Walk({0x1031b6968?, 0x140029f0e60?}, {0x1031b7750, 0x14006148e60})
	/opt/homebrew/Cellar/go/1.22.1/libexec/src/go/ast/walk.go:366 +0x2e14
go.uber.org/cff/internal.astWalk(...)
	/Users/izhuer/Code/cff/internal/ast.go:16
go.uber.org/cff/internal.(*compiler).compileFile(0x14001e89100, 0x14006148e60, 0x14001e890c0)
	/Users/izhuer/Code/cff/internal/compile.go:109 +0x178
go.uber.org/cff/internal.(*compiler).CompileFile(0x14001e89100, 0x26?, 0x0?)
	/Users/izhuer/Code/cff/internal/compile.go:96 +0x20
go.uber.org/cff/internal.(*Processor).Process(0x1400388bcb8, 0x14001e890c0, 0x14006148e60, {0x140040a8810, 0x26})
	/Users/izhuer/Code/cff/internal/process.go:29 +0xe0
main.run({0x14000134010?, 0x0?, 0x1400004e738?})
	/Users/izhuer/Code/cff/cmd/cff/main.go:140 +0x488
main.main()
	/Users/izhuer/Code/cff/cmd/cff/main.go:79 +0x70

I wonder if the compiler could handle this case more gracefully, e.g., perhaps by returning a meaningful error like "cannot infer constant value from non-const expression", instead of panicking.

Would love to hear your thoughts, and I am very happy to dig in further or open a PR if it helps!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions