Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failing codegen on Any #528

Open
delaneyj opened this issue Jun 27, 2023 · 4 comments
Open

Failing codegen on Any #528

delaneyj opened this issue Jun 27, 2023 · 4 comments
Labels
P3 Low: Not priority right now

Comments

@delaneyj
Copy link

ts, err := ipld.LoadSchemaBytes([]byte(`

type Foo struct {
    a Int
    b Int
    c String
}

type Bar struct {
    d Bytes
    e Bool
}

type Baz union {
    | Foo "foo"
    | Bar "bar"
} representation keyed
	`))

	if err != nil {
		log.Fatal(err)
	}

	gengo.Generate("xxx", "pkgname", *ts, &gengo.AdjunctCfg{})

Panics at...

panic: add more type switches here :), failed at type Any
  1. I would think Generate should error, not panic.
    2, Is the generator in a good state or is it not up to date with the current state of IPLD? I noticed the CLI tool was archived.
@rvagg
Copy link
Member

rvagg commented Jun 28, 2023

Any is much newer than most of the codegen code, but it's also a massive addition to any typed implementation because of how many things it touches and how much testing it needs.

As mentioned on the bridged #ipld community channel, codegen has been deprioritised for now with most focus put in to bindnode as our primary typed implementation for now.

Someone could put in some work to codegen and submit a PR but I couldn't promise to be timely with a review unfortunately because of how big it all is—although lots of tests would certainly help.

As for its general usability - if you avoid the unimplemented features then it works just fine, it's in active use across a lot of projects - notably in the dag-pb codec itself: https://github.com/ipld/go-codec-dagpb and the unixfs ADL: https://github.com/ipfs/go-unixfsnode/tree/main/data; it's just got a lot of unimplemented edges, Any being a rather big one unfortunately.

@rvagg rvagg added the P3 Low: Not priority right now label Jun 28, 2023
@rvagg rvagg moved this from 🥞 Todo to 🗄️ Backlog in IPLD team's weekly tracker Jun 28, 2023
@delaneyj
Copy link
Author

Just so I'm being clear, I'm not using Any, at least directly. The generator fails as it sets up internal maps for callbacks. From what I can see it never gets to my typedefs. I'm surprised any code will work in is current state.

I appreciate you don't have the cycles and it's not a priority, thank you for the honest response.

@rvagg
Copy link
Member

rvagg commented Jun 29, 2023

The way to use codegen isn't with a parseable schema (yet), it predates schema parsing in fact and this is the problem you're running in to - parsed schemas end up pulling in a lot of stuff that codegen can't handle even without touching them.

The way to use codegen is to build the schema programmatically. You can see that in action here: https://github.com/ipld/go-codec-dagpb/blob/master/gen.go and https://github.com/ipld/go-codec-dagpb/blob/master/gen.go

Also if you want to see a codegen to bindnode transition then check out ipld/go-ipld-adl-hamt#39 which took the older gen.go approach linked above and replaced it with a parsed schema file and bindnode types; removed a ton of code in the process and made it much cleaner.

@warpfork
Copy link
Collaborator

warpfork commented Jun 29, 2023

This probably happens because the "prelude" was revamped to use Any, despite the codegen's lack of support for it.

You can see where that's introduced here:

ts.Accumulate(schema.SpawnAny("Any"))

The codegen should work fine with schemas parsed from the DSL. The only hiccup is that that parsing pipeline passes through the Compile function linked above, so it always gets you the prelude types. (I guess the programmatic route doesn't do so.)

Depending on what your aims are, this may provide an alternative path to hacking towards what you want: We did always mean to have the prelude be optional, just never introduced a flag for that :) You can see how that flag would be pretty easy to introduce, though. Adding Any to the codegen would of course be wonderful, but might be a larger piece of work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 Low: Not priority right now
Projects
Status: 🗄️ Backlog
Development

No branches or pull requests

3 participants