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

Build broken since rustc nightly-2017-03-04 #80

Closed
sanmai-NL opened this issue Mar 14, 2017 · 9 comments
Closed

Build broken since rustc nightly-2017-03-04 #80

sanmai-NL opened this issue Mar 14, 2017 · 9 comments

Comments

@sanmai-NL
Copy link
Contributor

libsyntax changes again ... I've been working on fixes myself for a while today, but the required set of changes appeared to grow quickly, and I wouldn't be able to fix them efficiently without being very familiar with libsyntax and maud internals.

   Compiling maud_macros v0.16.2
error[E0425]: cannot find function `tts_to_parser` in module `parse`
   --> /home/sanmai/.cargo/bin/registry/src/github.ghproxy.top-1ecc6299db9ec823/maud_macros-0.16.2/src/parse.rs:126:33
    |
126 |         let mut parser = parse::tts_to_parser(self.render.cx.parse_sess, tts);
    |                                 ^^^^^^^^^^^^^ not found in `parse`

error[E0053]: method `fold_tt` has an incompatible type for trait
   --> /home/sanmai/.cargo/bin/registry/src/github.ghproxy.top-1ecc6299db9ec823/maud_macros-0.16.2/src/parse.rs:91:5
    |
91  |     fn fold_tt(&mut self, mut tt: &TokenTree) -> TokenTree {
    |     ^ expected enum `syntax::tokenstream::TokenTree`, found reference
    |
    = note: expected type `fn(&mut parse::FlattenNtFolder, syntax::tokenstream::TokenTree) -> syntax::tokenstream::TokenTree`
               found type `fn(&mut parse::FlattenNtFolder, &syntax::tokenstream::TokenTree) -> syntax::tokenstream::TokenTree`
@bb010g
Copy link
Contributor

bb010g commented Mar 14, 2017

Last version that compiles for me is nightly-2017-03-04.

@lambda-fairy
Copy link
Owner

lambda-fairy commented Mar 17, 2017

Thanks for reporting! Sadly, we're in an annoying intermediate state right now. The latest changes pretty much force us to use the new procedural macro interface (rust-lang/rust#38356), but parts of this new system aren't finished yet.

My recommendation at this point is to pin to nightly-2017-03-04 (thanks @bb010g) and wait for the dust to settle.


As far as I know, the only thing blocking a port is this task:

  • Implement a minimal API for proc_macro::TokenStream as outlined in the RFC.

The current parser, which performs direct pattern matching on &[TokenTree], will not work with the new interface. So we need a good replacement before switching over.

There are also these nice-to-haves:

  • Include a TokenStream quoter proc_macro::quote! behind the proc_macro feature gate.

This isn't a hard requirement, but the codegen would be nicer with this around.

  • Provide a way for proc_macro authors to create expansions that use items in a predetermined crate foo without requiring the macro user to include extern crate foo; at the crate root.

This also isn't a hard requirement. But this feature might affect the way Maud is designed, so I want to see how it goes before committing to a 1.0 release.

Answered at rust-lang/rust#38356 (comment)

Maud will pretty much go with what that comment describes.

@sanmai-NL sanmai-NL changed the title Build broken since about rustc 1.17.0-nightly (fd182c401 2017-03-13) Build broken since rustc nightly-2017-03-04 Apr 14, 2017
@sanmai-NL
Copy link
Contributor Author

sanmai-NL commented Apr 18, 2017

@lfairy:

I've been heavily using your crate for some time now and I'm using it in product prototyping. That it's stuck on an older nightly is a pressing problem (just reiterating, I assume everyone sees this). So maybe now is the time to reconsider the whole design of maud.

I find myself wondering whether being a compiler plugin is a good basis for maud. Could we leave this design behind and write a static HTML templating crate that uses more stable and simple patterns (e.g. the newtype pattern)? This would add more compile time validation possibilities over the present implementation (e.g. what tags exist and in what contexts are they allowed), without much extra work. I think such a new design will also make contributions easier, since Rust programmers do not need to be deeply invested in the (IMO poorly documented) compiler plugin architecture.

I suppose sometimes this kind of alternative design has crossed your mind or has been proposed to you. Could you please share your view on this?

@skariel
Copy link

skariel commented Apr 19, 2017

@lfairy unfortunalety Rocket requires a newer version of rust (>nightly-2017-03-04)

so maud cannot currently be used with Rocket... too bad because I like both

@skariel
Copy link

skariel commented Apr 19, 2017

using an older version of Rocket and pinning rust could solve this, however the Rocket extension of maud uses a too-recent Rocket version.

I'm submitting a PR to fix this. So using latest maud with pinned rustc and rocket 0.2.2 should work

@ernestas-poskus
Copy link
Contributor

@skariel

You can instruct Cargo.toml to look for local crate instead of remote one then the fix is not necessary.

maud = { path = "../rust/maud/maud", features = ["iron"] }

@lambda-fairy
Copy link
Owner

Good news!

I had another look at the proc macro interface. Turns out the current TokenStream API is more fleshed out than I thought. In particular, I can get a Vec<TokenTree> out of this by calling .collect() on the Cursor iterator. That should keep us going at least until rust-lang/rust#40939 is merged.

I will push this fix out later today.

@lambda-fairy
Copy link
Owner

lambda-fairy commented Apr 22, 2017

@sanmai-NL

I find myself wondering whether being a compiler plugin is a good basis for maud. Could we leave this design behind and write a static HTML templating crate that uses more stable and simple patterns (e.g. the newtype pattern)?

This idea has already been implemented as Domafic.

I also thought about building a solution around closures (like Markaby) but didn't have the time to explore this. I wonder how ergonomic this would be, especially when move vs non-move closures are involved.

This would add more compile time validation possibilities over the present implementation (e.g. what tags exist and in what contexts are they allowed), without much extra work.

This can be implemented in Maud as well. Since the macro has access to the full syntax tree at expansion time, it can emit warnings then. The proc macros 2.0 interface doesn't expose diagnostics support at the moment, but from my understanding it's on the roadmap.

I think such a new design will also make contributions easier, since Rust programmers do not need to be deeply invested in the (IMO poorly documented) compiler plugin architecture.

What Maud does, on a fundamental level, isn't hard to understand. Most of the difficulty is in the compiler plugin interface, which (as you say) isn't well designed or documented. Proc macros 2.0 should address both of these concerns.

A fully type-driven solution also introduces its own complexity. Since everything needs to be recorded in the type, you end up with large type signatures that often can't be written down. This leads to fiddling with closures or impl Trait, which both have their own corner cases. (I believe the Domafic docs mention this.) Overall, this doesn't seem more or less complex than a macro solution, and I don't see a clear "winner" in this space.

@lambda-fairy
Copy link
Owner

Released as maud_macros 0.16.3.

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

5 participants