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

Disabling of -Zunstable-options --pretty=expanded breaks cbindgen #43697

Open
jrmuizel opened this issue Aug 6, 2017 · 6 comments
Open

Disabling of -Zunstable-options --pretty=expanded breaks cbindgen #43697

jrmuizel opened this issue Aug 6, 2017 · 6 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.

Comments

@jrmuizel
Copy link
Contributor

jrmuizel commented Aug 6, 2017

cbindgen needs to parse macro expanded rust code in order to generate C/C++ bindings. The removal of --pretty=expanded breaks this. What's the suggested path forward here? Not having this forces binding generation to be done with a nightly tool chain and the results checked in to the tree instead of being done at build time.

@nagisa nagisa added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 7, 2017
@nagisa
Copy link
Member

nagisa commented Aug 7, 2017

The approach for you would be to ask for a stable flag for this of some sort. We’ve already stabilised a few on request, e.g. --emit=mir.

@staktrace
Copy link
Contributor

See also #43364.

@Mark-Simulacrum Mark-Simulacrum added C-bug Category: This is a bug. C-feature-request Category: A feature request, i.e: not implemented / a PR. and removed C-bug Category: This is a bug. labels Aug 10, 2017
@nrc nrc added the T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. label Aug 10, 2017
@nrc
Copy link
Member

nrc commented Aug 10, 2017

We really should not stabilise --pretty it is a bit of a mess. The least bad CLI option seems to be --emit=expanded to emit the macro-expanded code. However, I also don't like this - the generated code is not guaranteed to compile (because hygienic names are not reflected in the generated source text), it means a big chunk of compiler code (pretty printing) is now exposed to stable users, and I worry about bugs, etc. in that code.

I think an ideal solution is that cbindgen should not be looking at expanded code, but at the AST post-expansion (or the TokenStream, though that is not easy right now) - I assume cbindgen has to do some parsing of its own? However, that is also unstable (and actually likely to change, c.f., the output of --pretty expanded), so not very practical.

Another solution might be using a shim compiler driver - this is what the RLS will do - see https://github.com/nrc/rls-rustc. That would mean that building cbindgen would need a nightly compiler, but running it (i.e., compiling webrender or whatever) would work with a stable toolchain (unless the output of --pretty expanded changes between versions).

How does the shim solution sound?

@nrc
Copy link
Member

nrc commented Aug 21, 2017

@jrmuizel do either of the above solutions sound reasonable to you?

@jrmuizel
Copy link
Contributor Author

I think we should be able to work something out.

@nrc nrc removed the I-nominated label Aug 22, 2017
@steveklabnik
Copy link
Member

Triage: this is a very old issue. was something worked out? Can we close it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants