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

avoid using "C.enum_" expressions in the generated go code #150

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jstrachan
Copy link

if the C header file uses C enums then we can generate go code of the form C.enum_ which doesn't compile; so using something simpler like uint32 fixes it

this workaround works great for me on a project I'm using c-for-go on. I wonder if we need some configuration to enable this behaviour?

if the C header file uses C enums then we can generate go code of the form `C.enum_` which doesn't compile; so using somethign simpler like `uint32` fixes it

this workaround works great for me on a project I'm using c-for-go on. I wonder if we need some configuration to enable this behaviour?
@xlab
Copy link
Owner

xlab commented May 26, 2023

Can you please provide an example enum that will produce broken code? The enums subject is very tricky as the size is dynamic, if you have enum with -1, C compiler may wrap it into 255 as a byte, and Go runtime will wrap uint32 at -1 into something much bigger (4294967295) or won't compile at all, because constants are not overflowing.

@jstrachan
Copy link
Author

here's a little sample project that demonstrates the issue:
cheese.zip

if you run make all you'll see the compile error:

$ make all
c-for-go -out .. cheese.yml
  processing cheese.yml done.
go test ./...
# github.com/jstrachan/cheese
cgo-dwarf-inference:1:17: error: expected identifier or '{'
__typeof__(enum ) *__cgo__0;
                ^
cgo-dwarf-inference:1:28: error: type name requires a specifier or qualifier
__typeof__(enum ) *__cgo__0;
                           ^
cgo-dwarf-inference:1:28: error: expected ')'
cgo-dwarf-inference:1:11: note: to match this '('
__typeof__(enum ) *__cgo__0;
          ^
3 errors generated.

which is basically due to this code in the generated cheese.go file:

func FieldType_ToString(_type C.enum_) string {

@jstrachan
Copy link
Author

I wonder if the issue I'm hitting is more to do with the use of an enum thats not using a typedef

@xlab
Copy link
Owner

xlab commented May 27, 2023

Related discussion #127 (comment)

@jstrachan
Copy link
Author

I'm afraid I can't quite grok how that comment affects a CEnumSpec having no Tag or Typedef and so generating the type expression C.enum_ for a go type which doesn't compile....

did you mean we need some logic to figure out the size of the enum to determine a more reliable go type than uint32? In the above sample project the CEnumSpec has the values of the enum we know the maximum value of the enum.

I'm just not sure without something like this patch to get all those use of CGoName() to not use C.enum_

@xlab
Copy link
Owner

xlab commented May 30, 2023

The comment I referenced was about how to extract the proper size for enums, so it won't overflow anything. I think that assuming that every enum value can be treated as return "uint32" is not fail-proof enough :)

@jstrachan
Copy link
Author

@xlab thanks for the clarification!

So I'm wondering if we need a configuration option for a strategy on how to pick the right size of the enum given the number of members?

I notice that typedef'd enums in the header file seem to be int32 for me.

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