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

Dependence on fmt::Debug #296

Closed
kornelski opened this issue Apr 20, 2024 · 2 comments
Closed

Dependence on fmt::Debug #296

kornelski opened this issue Apr 20, 2024 · 2 comments

Comments

@kornelski
Copy link
Contributor

I'm proposing an option to disable fmt::Debug, mainly to reduce executable sizes, and to thoroughly strip symbol names from executables.

When testing it, it quickly became apparent that thiserror uses fmt::Debug, and this is a common pattern in proc macros.

impl ToTokens for Trait {
    fn to_tokens(&self, tokens: &mut TokenStream) {
        let trait_name = format_ident!("{}", format!("{:?}", self));
        tokens.extend(quote!(::core::fmt::#trait_name));
    }
}

I wanted to check how you feel about an option "breaking" fmt::Debug. Shall fmt::Debug be guaranteed to work in proc-macros? Would you be okay changing the implementation to use something else to stringify enums? (strum's Display, or manual match … => "…"). I'm also proposing to add #[cfg(debug_fmt_detail = "full")] to let crates use fmt::Debug when it's on, and fall back to something else when it's a no-op.

@dtolnay
Copy link
Owner

dtolnay commented Apr 20, 2024

As an opt-in compiler flag, I think it's fine that it causes some proc macros to not work. I don't think special consideration needs to be built into the option to behave differently in proc macros.

That said, motivation number 1 in rust-lang/rust#123940 (comment) does not apply to proc macros. If motivation 1 is most of the sincere motivation for this option, and motivation 2 is much less valuable to anyone in practice, then it might be nice if you could make {:?} behave normally in proc macros.

That sounds hard though, at least when Cargo's "cross compile mode" is not in effect (i.e. when --target is not passed). For crates that are a dependency of both macro and non-macro code, it's not necessarily worth making them get compiled twice just to have different {:?} behavior. (As I understand it, in "cross compile mode" they're built twice already anyway, even if host and target are identical targets.) So if you do add a carve-out for proc macros but it only applies when --target is used, I don't see that being valuable because it won't substantially reduce the need for proc macros to adjust their code accommodating this new option.

@kornelski
Copy link
Contributor Author

Thanks for the response and the code update.

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

2 participants