Skip to content
This repository has been archived by the owner on Dec 27, 2022. It is now read-only.

(WIP) Proc macro stable #18

Closed
wants to merge 3 commits into from

Conversation

softprops
Copy link
Contributor

The end result of this should hopefully be the follow up on #17 (review) and close out #16

I expect I'll have a lot of questions so I'm going to let this pr be a point of reference with a checklist to keep track of where I am along the way

  • drop proc-macro2
  • set min supported rust version in travis to 1.30.0
  • drop proc-macro-hack
  • convert mashup to function like proc macro?
  • profit (able to to use 2018 imports)

@softprops
Copy link
Contributor Author

softprops commented Oct 30, 2018

@dtolnay okay I feel like I'm already getting stuck on groking the next two steps. I'm reading through the docs for proc-macro-hack and I'm already feeling like I'm a little in over my head.

Maybe its best to summaries the pieces of mashup I understand and then start questions from there.

Essentially mashup is a bit like macro inception. Its a macro that generates macros out of necessity for collecting statements declaring components that should be concatenated. I think I understand the latter part more than the former.

In the collection phase the impl's parse fn is called and collects a name it will use as an exported new macro name, m! in most examples, a string reference which will be replaced the concatenated pieces after {tag}["name"] = Having dug though that already I have a decent understanding of those that works.

Before we get there's a declaration site and impl site where some magic happens after what I assume to be a whittling down of a block of syntax down to a single line of syntax in mashup and mashup_parser

Work gets handled off to the actual impl here. There's some interaction that I don't quite understand that enables a form of derive macro which is the hack part of the proc-macro-hack crate that has my head spinning.

What I was initially thinking was that I could skip the need for understanding that and go straight to the transition to stable proc macro.

My initial approach was ( in the impl crate ) to create a new entrypoint for that would also delegate to the parse method.

Those interfaces typically take two streams and return a new TokenStream. I don't think I need the args so I'm discarding that and focusing on input

here's a rough sketch of what I think that may look like

#[proc_macro_attribute]
pub fn mashup_macro_stable(_: TokenStream, input: TokenStream) -> TokenStream {
    let input = parse(input);

    let mut macros = String::new();
    for (name, concat) in input {
        macros += &make_macro(name, concat);
    }
    macros.parse().unwrap()
}

next up is figuring out how to wire that into the consumer ( mashup ) crate.
this is kind of where I'm getting stuck. I'm assuming I'd need to do that here

with something like

    (@begin ($($parse:tt)*)) => {
        #[mashup_macro_stable]
        $($parse)*;
    };

but that doesn't quiet work as I'll get an error like the following

   Compiling mashup v0.1.9 (/Users/dougtangren/code/rust/mashup)
error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found `b`
 --> tests/test.rs:7:20
  |
7 |         m["k"] = a b c;
  |                    ^ expected one of 8 possible tokens here

error[E0658]: attributes on expressions are experimental. (see issue #15701)
 --> tests/test.rs:6:5
  |
6 | /     mashup! {
7 | |         m["k"] = a b c;
8 | |     }
  | |_____^

I feel like I'm on the brink of understanding here am also feeling a little stuck.

Can you drop another hint/point of direction form what I described above? If it would be helpful I could commit what I have so you can see the errors I'm looking at in travis.

@dtolnay
Copy link
Owner

dtolnay commented Oct 30, 2018

After dropping proc-macro-hack there is no longer a need for separate declaration crate and implementation crate. There would be just one crate mashup containing:

#[proc_macro]
pub fn mashup(input: TokenStream) -> TokenStream {
    /* ... */
}

which takes in the mashup! input and emits the right substitution macro(s).

@softprops
Copy link
Contributor Author

Oh this may be simpler that I thought! I'll give this another look tomorrow

@softprops
Copy link
Contributor Author

doh! at least I thought that would be simple. Now running into another issue that seems like it's by design. It seems that procedural macros cannot expand to macro definitions that seems kind of like a show stopper for porting this to a first class proc macro because expanding to macro definitions is this macros specialty!

@dtolnay
Copy link
Owner

dtolnay commented Oct 31, 2018

Ah right. Time for some more indirection! Procedural macros in item position can expand to invocations of derive macros, and derive macros can expand to macro_rules macro definitions.

mashup! {
    m[...] = ...;
    qrst[...] = ...;
}

// expands to

#[derive(mashup::EnumHack)]
enum _mashup_1m_4qrst { // some mangled name based on the substitution macro names
    Value = (stringify! {
        macro_rules! m {
            ...
        }
        macro_rules! qrst {
            ...
        }
    }, 0).1,
}

// expands to

macro_rules! m {
    ...
}
macro_rules! qrst {
    ...
}

@softprops
Copy link
Contributor Author

@dtolnay I have no idea what you look like in real life but I can only imagine you wear a purple a wizard hat. I'll take a shot at that approach

@softprops
Copy link
Contributor Author

we may need one more level of indirection :/

I'm now seeing "error[E0658]: procedural macros cannot be expanded to statements (see issue #38356)". I think that's seems to come from rustc here.

for ctx here's what I switched to returning from the new mashup! proc macro.

  format!(r#"
        #[derive(mashup::EnumHack)]
        enum {} {{
            Value = (stringify! {{
                {}
            }}, 0).1,
        }}
    "#, dummy, macros)

I'm also borrowing some of the source from the proc macro hack repo. In particular this Parse impl which actually depends on some proc macro2 types. Likely because Parse hasn't been implemented yet for proc_macro2::{TokenStream,TokenTree}

likewise Im starting to understand a bit about the proc macro hack crate as it uses a trick similar to the one I'm trying to perform

#[doc(hidden)]
#[proc_macro_derive(EnumHack)]
pub fn enum_hack(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
    let inner = parse_macro_input!(input as EnumHack);
    proc_macro::TokenStream::from(inner.token_stream)
}

@softprops
Copy link
Contributor Author

It may also be an issue with the call syntax. There's some references here that describe what you can and can't do with proc macros

@dtolnay
Copy link
Owner

dtolnay commented Nov 1, 2018

I came up with a fresh batch of hacks today (as you noticed in the proc-macro-hack repo) which makes it possible to use function-like procedural macros in expression position. That makes it so that we may no longer need substitution macros at all. I put together a prototype of an approach without substitution macros in https://github.com/dtolnay/paste. Could you take a look at whether it would work for your use case? I believe it behaves correctly with respect to 2018-style macro imports as far as I have been able to test.

@softprops
Copy link
Contributor Author

Holy smokes. I'll try this out tomorrow in an app tomorrow and let you know. In learned alot about macros in the last few weeks. Thank you so much for being patient with me.

@softprops
Copy link
Contributor Author

@dtolnay did a test run. Over here https://github.com/softprops/lando/pull/38/files the first thing I noticed (which may just be me miss understanding new crate resolution semantics ) was that a consuming crate doesn't automatically get transient reference paste so I may need to re-export from my crate as I would pre 1.30.0

below is the output from a consumer of my crate ( which depends on paste::expr! )

error[E0433]: failed to resolve. Use of undeclared type or module `paste`
  --> src/lib.rs:7:1
   |
7  | / gateway!(|request, _| {
8  | |     println!("{:?}", request.path_parameters());
9  | |     Ok(lando::Response::new(format!(
10 | |         "hello {}",
...  |
16 | |     )))
17 | | });
   | |___^ Use of undeclared type or module `paste`

@softprops
Copy link
Contributor Author

softprops commented Nov 1, 2018

after re-exporting

pub use paste::expr;

I'm seeing a different error. It may be due to me using the wrong paste method.

error: macro expansion ignores token `{` and any following
 --> <::paste::expr macros>:2:1
  |
2 | {
  | ^
  |
note: caused by the macro expansion here; the usage of `expr!` is likely invalid in item context
 --> src/lib.rs:7:1
  |
7 | / gateway!(|request, _| {
8 | |     println!("{:?}", request.path_parameters());
9 | |     Ok(lando::Response::new(format!(
10| |         "hello {}",
... |
16| |     )))
17| | });
  | |___^
  = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

error: aborting due to previous error

below is the what my paste usage looks like

        expr! {
          gateway! { @module ([<lib env!("CARGO_PKG_NAME")>],[<initlib env!("CARGO_PKG_NAME")>], [<PyInit_lib env!("CARGO_PKG_NAME")>])
                  @handlers ($($handler => $target),*) }
        }

@softprops
Copy link
Contributor Author

when I try with item! I get the following error message

---- src/lib.rs - gateway (line 233) stdout ----
error[E0658]: procedural macros cannot be expanded to statements (see issue #38356)

@dtolnay
Copy link
Owner

dtolnay commented Nov 1, 2018

Looks like you are missing a $crate::. It worked when I tried it as:

helper/src/lib.rs

extern crate paste;

#[doc(hidden)]
pub use paste::expr as paste_expr;

#[macro_export]
macro_rules! helper {
    () => {
        $crate::paste_expr! {
            [<lib env!("CARGO_PKG_NAME")>]
        }
    };
}

testing/src/main.rs

extern crate helper;

fn libtesting() {
    println!("success");
}

fn main() {
    helper::helper!()();
}

@softprops
Copy link
Contributor Author

I was able to work around the scoping issue but now I'm stuck on this expr/item error here

It may have something to with the expansion order or something

error: macro expansion ignores token `{` and any following
 --> <::paste::expr macros>:2:1
  |
2 | {
  | ^
  |
note: caused by the macro expansion here; the usage of `expr!` is likely invalid in item context
 --> src/lib.rs:7:1
  |
7 | / gateway!(|request, _| {
8 | |     println!("{:?}", request.path_parameters());
9 | |     Ok(lando::Response::new(format!(
10| |         "hello {}",
... |
16| |     )))
17| | });
  | |___^
  = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

@dtolnay
Copy link
Owner

dtolnay commented Nov 1, 2018

Is there test coverage of gateway!? I tried cargo test against softprops/lando@0f4cead which passed, and the CI build on softprops/lando#38 is passing. If not, what code should I be compiling to reproduce the failure?

the usage of `expr!` is likely invalid in item context

If gateway! isn't expanding to an expression, you are going to want paste::item! rather than paste::expr!.

@softprops
Copy link
Contributor Author

added an example branch on the repo i'm testing in here softprops/serverless-lando#6

@softprops
Copy link
Contributor Author

what's strange is that demo app is essential the same as what should be getting picked up by a doc test here

//! ```rust
//! # #[macro_use] extern crate lando;
//! # fn main() {
//! gateway!(|_request, context| {
//!     println!("👋 cloudwatch logs, this is {}", context.function_name());
//!     // return a basic 200 response
//!     Ok(lando::Response::new(()))
//! });
//! # }
//! ```

@dtolnay
Copy link
Owner

dtolnay commented Nov 1, 2018

Since gateway! is expanding to an item not an expression, you want paste::item.

- pub use paste::expr as paste_expr;
+ pub use paste::item as paste_item;

-         $crate::paste_expr! {
+         $crate::paste_item! {

After that I see error[E0599]: no method named `cloned` found for type `std::option::Option<&str>` in the current scope on the call to .get("name").cloned() which seems correct to me from skimming the type signatures. Is that code that you would have expected to work?

@softprops
Copy link
Contributor Author

Thanks for all of the hand holding in this. I thought I had tried item! and it was causing my doc tests t o fail so I backed out. I turns out my doc tests were interestingly all wrong in a way because of a pattern I'd read about here where I had wrapped my macro doc examples in a hidden # fn main() . I think this had a side effect of treating the macro expansion as statements which explains was getting doc examples prior for

---- src/lib.rs - gateway (line 194) stdout ----
error[E0658]: procedural macros cannot be expanded to statements (see issue #38356)
  --> src/lib.rs:198:1
   |
5  | / gateway!(|request, _| {
6  | | Ok(lando::Response::new(format!(
7  | |       "hello {}",
8  | |        request
...  |
12 | |    )))
13 | | });
   | |___^

I moved all of my fake doc mains to a separate hidden line in examples # fn main() {}

I can confirm though that this fixed the issue for me. All of that for the ability to not have to #[macro_use] mashup whew!

@softprops softprops closed this Nov 2, 2018
@softprops
Copy link
Contributor Author

here's what that last commit looked like for future reference softprops/lando@572e95e

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants