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

Use on stable Rust? #31

Closed
askeksa opened this issue May 5, 2018 · 25 comments
Closed

Use on stable Rust? #31

askeksa opened this issue May 5, 2018 · 25 comments

Comments

@askeksa
Copy link

askeksa commented May 5, 2018

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.

@CensoredUsername
Copy link
Owner

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.

@askeksa
Copy link
Author

askeksa commented May 5, 2018

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. :) 👍

@RReverser
Copy link

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.

@CensoredUsername
Copy link
Owner

CensoredUsername commented Oct 16, 2018

Yay, thanks for the headsup.

@CensoredUsername
Copy link
Owner

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.

@dtolnay
Copy link

dtolnay commented Nov 6, 2018

proc_macro_diagnostic

It looks like the only code using proc_macro_diagnostic is ::err. That would be better implemented by rendering errors as compile_error invocations in the output token stream, spanned with whatever span you want the error to appear on. This is similar to Syn's Error::to_compile_error.

proc_macro_hygiene

https://github.com/dtolnay/proc-macro-hack should entirely eliminate the need for this feature.

@CensoredUsername
Copy link
Owner

This is similar to Syn's Error::to_compile_error.

I didn't know about this approach, but it should be doable to implement without significant changes. Thanks!

https://github.com/dtolnay/proc-macro-hack

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!

@dtolnay
Copy link

dtolnay commented Nov 6, 2018

How are alias declarations currently done that is not stable?

@CensoredUsername
Copy link
Owner

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.

@CensoredUsername
Copy link
Owner

Hurray! We're now using proc-macro (with a few nightly features still required).

@MarkMcCaskey
Copy link

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!

@CensoredUsername
Copy link
Owner

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

@frol
Copy link

frol commented Mar 15, 2020

I am looking into resolving this. It seems that there are two unstable features in use:

  • proc_macro_diagnostic

    It seems that this is used for better compilation time error messages, which can be feature-gated for stable Rust with some degradation of the developer experience.

  • proc_macro_span

    It seems that it is used to handle file local data. It is unclear to me what is the use-case for the "file local data"; can a single DynasmData be used?

So far it seems that it is not a huge scope of work. I can give it a try.

@CensoredUsername
Copy link
Owner

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.

@frol
Copy link

frol commented Mar 16, 2020

It seems that some parts of proc_macro_hygiene (AFAIK, those are enough for dynasm) are going to be stabilized by Rust 1.44: rust-lang/rust#68717

@frol
Copy link

frol commented Mar 16, 2020

Well, it seems that I don't have enough experience to come up with an alternative solution to proc_macro_span use. Can there be an uglier version of the API, which would target stable Rust?

@CensoredUsername
Copy link
Owner

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.

@CensoredUsername
Copy link
Owner

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!

@CensoredUsername
Copy link
Owner

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.

@frol
Copy link

frol commented Jul 1, 2020

Does this mean that dynasm-rs 0.7.0 can be used without #![feature(proc_macro_hygiene)] with Rust 1.45 stable release? (the tests still require this feature)

@frol
Copy link

frol commented Jul 1, 2020

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

@CensoredUsername
Copy link
Owner

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

@CensoredUsername
Copy link
Owner

Slight headsup: We're now on 0.7.1 due to a small bug in dynasmrt::Modifier.

@CensoredUsername
Copy link
Owner

Currently awaiting dtolnay/proc-macro2#241 to happen

@CensoredUsername
Copy link
Owner

dynasm-rs 1.0.0 has been released targetting rustc 1.45 stable 🎉

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

No branches or pull requests

6 participants