displaydoc icon indicating copy to clipboard operation
displaydoc copied to clipboard

Future incompatibility with Rust RFC 3373: Avoid non-local definitions in functions

Open Urgau opened this issue 1 year ago • 1 comments
trafficstars

Rust RFC 3373: Avoid non-local definitions in functions was accepted and it's implementation at https://github.com/rust-lang/rust/pull/120393 found that this crate would be affected by it.

To be more precise users of this crate would be affected by it, in the form of a warn-by-default lint: non_local_definitions. This is because the derive macros from this crate use impl in a local context, const _DERIVE_Display_FOR_???:

https://github.com/yaahc/displaydoc/blob/f0b62a55ec2495b1a60b18f1d93f8b27e53123a7/src/expand.rs#L22-L25

Fortunately a simple fix exist for this crate, by using a const-anon instead of named one:

-    let dummy_const = format_ident!("_DERIVE_Display_FOR_{}", input.ident);
     Ok(quote! {
         #[allow(non_upper_case_globals, unused_attributes, unused_qualifications)]
-        const #dummy_const: () = {
+        const _: () = {

I would suggest applying some form of the patch above as well as releasing a patch version of this crate, as to have a fix available for users as soon as possible.

cc @yaahc

Urgau avatar Feb 04 '24 14:02 Urgau

The RFC impl is merged in the latest nightly. displaydoc dependent nighly crates are getting the warning now.

rnbguy avatar Feb 26 '24 13:02 rnbguy

Does that patch actually work? My understanding is that displaydoc does this scoping so that it can have some local types and impls

Manishearth avatar Jun 18 '24 19:06 Manishearth

Yes, it should work. We do (in rustc) special case const-anon to not lint, given the widespread usage of that pattern.

Urgau avatar Jun 18 '24 20:06 Urgau

But it seems like that just hoists all the definitions to the outer scope? So they'll clash with each other?

Manishearth avatar Jun 18 '24 20:06 Manishearth

This should work: https://github.com/yaahc/displaydoc/pull/49

Manishearth avatar Jun 18 '24 21:06 Manishearth

But it seems like that just hoists all the definitions to the outer scope? So they'll clash with each other?

An anonymous const item with a block expression still introduces a new scope just like a named const. (In fact, it's the block expression that's doing all the work; the const _: () = is just a way to put a const-evaluated expression in a context that accepts items.)

kpreid avatar Jun 18 '24 21:06 kpreid

@kpreid hmm, in that case the error message for this future incompat lint seems incorrect?

https://github.com/rust-lang/rust/issues/120363#issuecomment-2177064702

Manishearth avatar Jun 18 '24 21:06 Manishearth