Skip to content

break lexgen cborgen circular dependence#716

Open
brianolson wants to merge 5 commits intomainfrom
bolson/lex-cbor-decircular
Open

break lexgen cborgen circular dependence#716
brianolson wants to merge 5 commits intomainfrom
bolson/lex-cbor-decircular

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
Contributor

@jazware jazware 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