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

break lexgen cborgen circular dependence #716

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

brianolson
Copy link
Contributor

OLD: When a new type comes in through lexgen, it cannot be compiled until after cborgen runs, but cborgen cannot run because the type cannot be compiled. So comment out a section, run cborgen, then uncomment the hidden code.

New: make lexgen; make cborgen; make test done
A type check that used to be done at compile time (which would fail) is now done in unit test code, which will pass after cborgen is run.

Requires forthcoming cbor-gen PR whyrusleeping/cbor-gen#102

@brianolson brianolson marked this pull request as ready for review August 7, 2024 18:07
Copy link
Collaborator

@ericvolp12 ericvolp12 left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me, I'm interested in testing it out a bit to make sure we don't regress here though so I might pull this version into some of my code that gets more hands-on with types that require CBOR marshalling/unmarshalling.

I'd wait for a @whyrusleeping approval on these changes though since I'm not 100% sure on the cbor dependency bits.

@brianolson
Copy link
Contributor Author

this doesn't handle a case I ran into later: lexgen for some types writes MarshalCBOR/UnmarshalCBOR if all their fields are objects known to lexgen. these implementations don't compile and still prevent cbor-gen from running and they need to be temporarily commented out. :-/

My other guess is that we split sources and use build flags. e.g. write Foo struct to foo.go and foo_cborgen.go flagged with //go:build !cborgen so that we can run cborgen with -tags cborgen and hide those sources during cborgen run.

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.

2 participants