-
Notifications
You must be signed in to change notification settings - Fork 54
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
Use on stable Rust? #31
Comments
Currently the plan is to wait for proper expression macros when they eventually arrive. There is currently no other solution that still allows dynasm to generate proper errors for invalid assembly, which I consider quite critical to a proper user experience. When expression macros eventually arrive on stable I'll gladly get it working on there. |
Good to hear there is a plan. This crate and its requirements is probably good input to the design discussions for expression macros. Looking forward to playing more with this. :) 👍 |
FWIW proc macros can be defined on stable Rust 1.29 now and it will be possible to use them on stable 1.30 (which is going to be released soon) so it might be worth to start migrating code over to new APIs. |
Yay, thanks for the headsup. |
I've ported the entire project to proc-macro on the dev branch and will be continuing development there. We'll have to be on nightly for a while still though since proc_macro_diagnostic and proc_macro_hygiene are not stabilized yet (the former necessary to give decent error messages, the latter to actually expand to expressions. However, I can now continue development of new architectures without worrying about having to redo their implementation, so work on the dev branch will continue for a while. |
It looks like the only code using proc_macro_diagnostic is
https://github.com/dtolnay/proc-macro-hack should entirely eliminate the need for this feature. |
I didn't know about this approach, but it should be doable to implement without significant changes. Thanks! Interesting. I'm having trouble understanding how it works but it looks promising. Outside of those I still need to find a solution for crate or file scoping of alias declarations, but it solves the largest issues I had to moving to stable. Thanks! |
How are alias declarations currently done that is not stable? |
In the old plugin system, alias declarations were scoped crate-local. This was done in a very hackish way (querying rustc which crate we were expanding in, and having a global static mapping of crate name to alias / current arch definition). Using proc-macro we don't have access to this information. I was thinking of changing alias definitions to be file-local (using information from Span::call_site()'s file name), but these methods were currently listed as semver-exempt. Note that this is currently not implemented yet, causing alias declarations to possibly be global which is something I'd like to avoid. |
Hurray! We're now using proc-macro (with a few nightly features still required). |
Hello! I'd also like if dynasm-rs worked on stable. Have things changed much over the last year? I may be able to work on this in my free time if there's a path forward! |
@MarkMcCaskey It's currently still a nightly-only lib. This is due to two reasons. The first is the use of Span information in the proc macro to emit accurate errors and handle file local data. The second is that proc macros expanding in statement or expression position is still unstable and requires the proc_macro_hygiene feature. Any help on getting that feature stabilized in rustc is greatly appreciated! |
I am looking into resolving this. It seems that there are two unstable features in use:
So far it seems that it is not a huge scope of work. I can give it a try. |
Hey there! You're right on the first point, that one could be easily feature gated. The second one is a bit more complex. File local data is a pretty nasty hack, but a useful one, to allow the user to declare the wanted architecture and supported features /register aliases once in a file instead of at every call site. Unfortunately there is no really nice way of doing this in the current proc macro model. Another reason to do it is to enforce separation of dynasm invocations between crates etc as due to the ugly hack nature of things that would otherwise not be guaranteed. Finally, there's a third issue : currently proc_macro_hygiene is still necessary to allow macro expansion in expression position. Even if you would solve the first two and it would build on stable it would be pretty useless unless that gets fixed. |
It seems that some parts of |
Well, it seems that I don't have enough experience to come up with an alternative solution to |
Ooh nice, that'd definitely bring us a lot closer to stabilization. There could be an uglier version of the API, which doesn't use the file-local data concept and only remembers things in the same invocation. To then specify things, the user could define their own macro using macro_rules! which would also contain all the alias/arch/feature settings they want. The one thing I'm very afraid of is that this will degrade error messages significantly unless in the meantime retokenization is fixed. Should be simple to test though. |
Progress: On the dev branch file-local data for alias / arch storage has been refactored into a feature. This means it's now possible to build the plugin on stable as long as that feature isn't used. The code in doc/examples has also been ported to not use that feature. Although it makes code slightly uglier, it's still useful without it thanks to macro hygiene improvements. I'm waiting for proc_macro2 to push a new version that exposes Span::mixed_site() hygiene to switch over to the new hygiene, and then it should be possible to use this crate on stable at the release of rust 1.45! |
I've just released version 0.7.0, which is a preview for what will be stable release v1.0.0 when rustc stable 1.45 releases. You can use it with a nightly 1.45 compiler right now to test with it, the only expected changes between 0.7.0 and 1.0.0 are the replacement of several Span::call_site() calls to Span::mixed_site() to take advantage of the new hygiene rules. |
Does this mean that dynasm-rs 0.7.0 can be used without |
FYI, I have created an issue on wasmer issue tracker with a proposal to upgrade dynasm-rs to 0.7 release (they currently use 0.5 release): wasmerio/wasmer#1488 |
@frol oh whoops, looks like I missed one feature call in the testing crate and as I could only test with a nightly compiler they still passed. I'll remove that, but it shouldn't affect the actual two crates. I'll fix that |
Slight headsup: We're now on 0.7.1 due to a small bug in dynasmrt::Modifier. |
Currently awaiting dtolnay/proc-macro2#241 to happen |
dynasm-rs 1.0.0 has been released targetting rustc 1.45 stable 🎉 |
Is there a plan to make the crate work on stable Rust at some point, i.e. using procedural macros (either via https://github.com/dtolnay/proc-macro-hack or using proper expression macros when they eventually arrive)?
I would like to use dynasm-rs for a JIT for modular sound synthesis, and I don't mind depending on unstable Rust to begin with, but it would be nice to have some assurance that it will work on stable eventually.
The text was updated successfully, but these errors were encountered: