Valast fails at runtime with type pointers

[krisnova]

Write now the type pointer generation is failing (at runtime!)

panic: interface conversion: interface {} is *int, not *int32

goroutine 1 [running]:
github.com/naml-examples/simple.(*App).Install(0xc000391200, 0xc000508000, 0x0, 0xa)
        /home/nova/src/tests/app.go:86 +0xed9
github.com/kris-nova/naml.Install(0x1afcaf0, 0xc000391200, 0xc0004a3bd0, 0x0)
        /home/nova/go/pkg/mod/github.com/kris-nova/naml@v0.2.6/cmd.go:386 +0xc7
github.com/kris-nova/naml.RunCommandLineWithOptions.func2(0xc0004d2000, 0x1, 0x1)
        /home/nova/go/pkg/mod/github.com/kris-nova/naml@v0.2.6/cmd.go:160 +0x39f
github.com/urfave/cli/v2.(*Command).Run(0xc000391440, 0xc0003fdf40, 0x0, 0x0)
        /home/nova/go/pkg/mod/github.com/urfave/cli/v2@v2.3.0/command.go:163 +0x4dd
github.com/urfave/cli/v2.(*App).RunContext(0xc00025bd40, 0x1afc930, 0xc00019a010, 0xc0001a4000, 0x2, 0x2, 0x0, 0x0)
        /home/nova/go/pkg/mod/github.com/urfave/cli/v2@v2.3.0/app.go:313 +0x810
github.com/urfave/cli/v2.(*App).Run(...)
        /home/nova/go/pkg/mod/github.com/urfave/cli/v2@v2.3.0/app.go:224
github.com/kris-nova/naml.RunCommandLineWithOptions(0x1afcaf0, 0xc000391200)
        /home/nova/go/pkg/mod/github.com/kris-nova/naml@v0.2.6/cmd.go:316 +0xf2b
github.com/kris-nova/naml.RunCommandLine(...)
        /home/nova/go/pkg/mod/github.com/kris-nova/naml@v0.2.6/cmd.go:51
main.main()
        /home/nova/src/tests/cmd/main.go:42 +0xa5

Looks like this is the culprit

            Replicas: valast.Addr(1).(*int32),

We should fix Valast to do something like this

            Replicas: valast.Addr(int32(1)).(*int32),

[krisnova]

Also int64() is afflicted by this

                    TerminationGracePeriodSeconds: valast.Addr(int64(30)).(*int64),

[fkautz]

I'll give a shot at fixing this tonight. I also have to send my previous patch upstream for packages to them to see what they say.

[krisnova]

i think we should be okay: https://twitter.com/krisnova/status/1421896990460243971?s=21

[fkautz]

Fantastic! Looking forward to collaborating with them. :)

[fkautz]

Are you already working on the conversion issues? I'm happy to take them on if you aren't already.

[krisnova]

go for it!!

[fkautz]

Some additional info:

simple use case works

    i64 := int64(3607)
    test_str := String(&i64)

yields

valast.Addr(int64(3607)).(*int64)

nested in struct fails

    i64 := int64(3607)
    got := str{
        ExpirationSeconds: &i64,
    }

    test_str := String(got)

yields

valast.str{ExpirationSeconds: valast.Addr(3607).(*int64)}

[fkautz]

https://github.com/hexops/valast/pull/15 https://github.com/hexops/valast/pull/16

I'll update github.com/fkautz/valast as well. Once these patches get in, I'll submit a patch to revert back to using hexops/valast.

[slimsag]

Just wanted to say thanks for finding these bugs :) Happy to see more folks getting use out of these packages.

I left some feedback on the PRs, just let me know if any of this exceeds your time budget for contributing - I'm happy to get more involved in fixing these issues if needed

BTW, interesting stuff with NAML, I'll be keeping this project in mind - maybe some interesting ways we could use it at my work @sourcegraph :)

[fkautz]

Oh cool, I know Beyang and Quinn from before they started sourcegraph. They are fun to work with. I didn't realize the connection here. Good stuff!

[krisnova]

Reading this made me smile - so happy to enable this conversation with the naml project - ill be following along closely

[krisnova]

ping - any update on this? was wondering if we could try to cut a 1.0.0 release and this would be a big part of that?

[fkautz]

I'm working on getting more feedback on an approach here. I suspect some of the generated tests in valast might not be valid.

In a nutshell, here is the problem:

vespera% cat main.go
package main

import "github.com/hexops/valast"
import "fmt"

func main() {
    type str struct {
        ExpirationSeconds *int32
    }

    broken := str{ExpirationSeconds: valast.Addr(3607).(*int32)}
    fmt.Println(broken)
}
vespera% go run main.go
panic: interface conversion: interface {} is *int, not *int32

goroutine 1 [running]:
main.main()
    /home/vespera/eh/main.go:11 +0x87
exit status 2

The go compiler just isn't smart enough to work out that valast.Addr(3607) should be a *int32 rather than *int based on the context.

[slimsag]

Happy to dig into this some more on my end if you end up in a state of "just want this fixed as a user of the library and don't care about contributing too much" BTW, just let me know :)

[krisnova]

Naml was able to vendor Valast 1.4.1 and with some small changes work around a minor issue

See https://github.com/kris-nova/naml/blob/main/codify/codify_test.go#L95-L113 See https://github.com/kris-nova/naml/blob/main/codify/codify.go#L232-L239 See https://github.com/kris-nova/naml/blob/main/go.mod#L7

I believe we can close this for now - feel free to reopen in the future if needed

[krisnova]

See https://github.com/hexops/valast/issues/20 for more