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

can't alias to existing D type #216

Closed
chriscamacho opened this issue Dec 16, 2018 · 15 comments
Closed

can't alias to existing D type #216

chriscamacho opened this issue Dec 16, 2018 · 15 comments
Labels

Comments

@chriscamacho
Copy link

chriscamacho commented Dec 16, 2018

when translating the header from https://github.com/ggambetta/libz80

dstep emitted the following code (in part)

alias ushort = ushort;
alias byte = ubyte;

fortunately its not a large header so I could just comment these alias's out and change any byte to ubyte ...

@ciechowoj
Copy link
Contributor

@chriscamacho The first case is easy to fix - just ignore the alias, but what about the second case? Do you think having dstep to automatically convert byte to ubyte would solve the problem?

@jacob-carlborg
Copy link
Owner

This looks like a case where DStep doesn't handle D keywords. A quick fix would be to just suffix the alias names with _ as we do in other places.

@chriscamacho
Copy link
Author

although in the C world _ is supposed to be reserved I haven't honestly seen it in practice, I'd say an initial scan to prefix any D keywords in the C source with _ or similar would be a good approach and would probably solve other undiscovered corner cases lurking to catch you out !

@jacob-carlborg
Copy link
Owner

We're currently adding a suffix, not a prefix, i.e byte -> byte_.

@ciechowoj
Copy link
Contributor

Yes, I guess the pragmatic approach with _ is fine. But it doesn't look 'nice', I'm wondering if renaming all bytes to ubytes isn't a better approach...

@chriscamacho
Copy link
Author

what if its not actually unsigned ?

@jacob-carlborg
Copy link
Owner

I'm wondering if renaming all bytes to ubytes isn't a better approach...

That seems quite brittle. What if byte is part of the API and now that symbol doesn't exist anymore and cannot be used by users of the binding.

@jacob-carlborg
Copy link
Owner

what if its not actually unsigned ?

Or not corresponding to D's ubyte but it's instead a typedef for int or whatever. (We need to account for stupid things).

@ciechowoj
Copy link
Contributor

If it's not actually unsigned then we can just ignore the alias as in the case of ushort above.

@jacob-carlborg
Copy link
Owner

What about code like this: typedef int byte?

@ciechowoj
Copy link
Contributor

ciechowoj commented Dec 31, 2018

Yes, I would rename it only if it has form like in the current bug. The stupid cases have to be resolved by other means (_ suffix for example).

@ciechowoj
Copy link
Contributor

Or in case of typedef int byte, rename all the bytes to ints

@ciechowoj
Copy link
Contributor

I wouldn't worried about the symbols, if naming of the types influenced the mangling we are screwed with any renaming (e.g. adding _). I don't think the aliases to primitive types are exported at all.

@jacob-carlborg
Copy link
Owner

I'm not worried about mangling. What I'm referring to is if the C header defines a symbol called byte and then the user tries to code like byte a = 0;, byte won't exist. But I guess we have the same problem with adding _.

@jacob-carlborg
Copy link
Owner

I guess the best would be to check all the usages of the typedef and then translate to what the typedef resolves to. This could be bundled in the existing --reduce-aliases flag.

jacob-carlborg added a commit that referenced this issue Jan 7, 2019
Fix #216: can't alias to existing D type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants