Support macros in renamed crate scenario

[zbraniecki]

What do you think about making this sub-crate only emit the bytes themselves, and wrapping the bytes in a normal macro_rules in the main crate, such that you can use $crate::TinyStr4 for better resolution behavior?

Originally posted by @sffc in #18

[zbraniecki]

I dove deeper into the issue reported by @sffc and as far as I can tell Rust does not have a solution to that at the moment.

The scenario is very edge-casey. It happens when you import the crate and rename it:

[dependencies]
foo = { package = "tinystr", path = "../../tinystr", features = ["macros"] }

In that scenario, you have tinystr imported as foo and then you can go to your lib.rs and do:

use foo::macros::tinystr4;

pub fn use_macro() {
    let x = tinystr4!("foo");
    assert_eq!(x, "foo");
}

#[cfg(test)]
mod tests {
    use super::*;
    #[test]
    fn it_works() {
        use_macro();
    }
}

You'd expect that to work, but since our code in macro does:

#[proc_macro]
pub fn tinystr4(input: TokenStream) -> TokenStream {
    let val = get_value_from_token_stream(input);
    let bytes: u32 = tinystr::TinyStr4::from_bytes(val.as_bytes())
        .expect("Failed to construct TinyStr from input")
        .into();

    let formula = format!("unsafe {{ tinystr::TinyStr4::new_unchecked({}) }}", bytes);
    formula.parse().unwrap()
}

it will not - it expects tinystr::TinyStr4 to point to the right place, but in our user crate we renamed it to foo so it should be foo::TinyStr4.

There is an issue on file for it rust-lang/rust#54363 and there's a workaround crate - https://crates.io/crates/proc-macro-crate which reads your Cargo.toml and finds the alias you used, but it's pretty heavy with a dependency on serde and toml.

I'd love to avoid that solution, and hence I'm proposing to consider using tinystr under a renamed alias with macros as out of scope.

I'm willing to keep this issue open and fix it when Rust introduces a solution, but for now, I believe that it is reasonable to assume that a rename scenario with proc-macro use is an edge case.

@sffc @Manishearth - would you be ok with that approach?

[Manishearth]

I don't get it: doesn't $crate work in a proc macro?

But yes, renamed crates are extremely rare and it's kinda typical for macros to break in that situation (not everyone remembers to use $crate) so I'd consider this out of scope

[sffc]

I'm okay declaring the problem out of scope, but I don't understand why we don't just fix it the right way.

[zbraniecki]

I don't get it: doesn't $crate work in a proc macro?

It seems it doesn't, or at least I can't get it to work. I tried doing what this PR does but with $crate::TinyStr4 and tried the same using quote crate, and am getting this token stream:

TokenStream [
    Ident {
        ident: "unsafe",
        span: #12 bytes(240..256),
    },
    Group {
        delimiter: Brace,
        stream: TokenStream [
            Punct {
                ch: '$',
                spacing: Alone,
                span: #12 bytes(240..256),
            },
            Ident {
                ident: "crate",
                span: #12 bytes(240..256),
            },
            Punct {
                ch: ':',
                spacing: Joint,
                span: #12 bytes(240..256),
            },
            Punct {
                ch: ':',
                spacing: Alone,
                span: #12 bytes(240..256),
            },
            Ident {
                ident: "TinyStr4",
                span: #12 bytes(240..256),
            },
            Punct {
                ch: ':',
                spacing: Joint,
                span: #12 bytes(240..256),
            },
            Punct {
                ch: ':',
                spacing: Alone,
                span: #12 bytes(240..256),
            },
            Ident {
                ident: "new_unchecked",
                span: #12 bytes(240..256),
            },
            Group {
                delimiter: Parenthesis,
                stream: TokenStream [
                    Literal {
                        kind: Integer,
                        symbol: "7303014",
                        suffix: Some("u32"),
                        span: #12 bytes(240..256),
                    },
                ],
                span: #12 bytes(240..256),
            },
        ],
        span: #12 bytes(240..256),
    },
]

which leads to this error:

error: expected expression, found `$`
  --> tests/macros.rs:12:16
   |
12 |     assert_eq!(tinystr4!("foo"), x);
   |                ^^^^^^^^^^^^^^^^ expected expression
   |
   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

As I said, the proc_crate_name works, its just heavy handed way to fix it.

but I don't understand why we don't just fix it the right way.

Because there is no "clean" fix. It's either a heavy dependency or a declarative macro handled in the tinystr library just to support the edge case.
I don't see much value in either, and expect Rust to eventually fix the core issue and then we can update the proc macro to handle the edge scenario.

What I think I see differently from you is that the solution you proposed is not, in my opinion, "just a fix". It's a quite elaborate and quirky way to add support for something that is a very uncommon scenario.
For that reason I'd prefer not to support that scenario than to maintain a quirk.

[sffc]

What I think I see differently from you is that the solution you proposed is not, in my opinion, "just a fix". It's a quite elaborate and quirky way to add support for something that is a very uncommon scenario.

Just to make sure we're on the same page: my "fix" is:

// In child crate
#[proc_macro]
pub fn tinystr4_bytes(input: TokenStream) -> TokenStream {
    let val = get_value_from_token_stream(input);
    let bytes: u32 = tinystr::TinyStr4::from_bytes(val.as_bytes())
        .expect("Failed to construct TinyStr from input")
        .into();
    let formula = format!("{}", bytes);
    formula.parse().unwrap()
}

// In main crate (untested)
macro_rules! tinystr4 {
    ($s:tt) => {
        unsafe { $crate::TinyStr4::new_unchecked(tinystr4_bytes!($s)) }
    }
}

I don't find this quirky or elaborate at all. I find it to be a clean separation between what is best done as a proc macro (performing a complicated calculation) and what is best done as a normal macro (interpolating the computed value into syntax).

[Manishearth]

That won't work because that's a cyclic dependency.

Interpolating computed values is pretty easy in both proc and regular macros.

[sffc]

This was fixed in #26.