Warning about non-local impl-definition in derive-macro output

[avl]

Code

I tried this code:

const _: () = {
    const _: () = {
        impl Callable for Dummy {}
    };
    pub trait Callable {}
    struct Dummy;
};

I expected to see this happen: The code should compile without errors or warnings.

Instead, this happened: There is a warning: "warning: non-local impl definition, impl blocks should be written at the same level as their item".

This might seem very convoluted. It is a minimization of code generated by the savefile-derive crate when it is generating implementations of AbiExportable for traits returning boxed Fn-trait objects. It generates helper-types for these, and then effectively recurses, generating impls for these helpers types. That's why we get two levels of the const _: () = {...}-trick. This trick is used by crates like savefile and serde, but I suppose serde doesn't end up recursing and is probably not affected by this issue.

Version it worked on

It works in stable rust 1.81, and also in rustc 1.83.0-nightly (52fd99839 2024-10-10).

Version with regression

It does not work in rustc 1.83.0-nightly (1bc403daa 2024-10-11).

@rustbot modify labels: +regression-from-stable-to-nightly -regression-untriaged

[jieyouxu]

cc @Urgau as you know about non_local_definitions.

EDIT: I'm dumb and was looking at different issue pages.

[avl]

Hmm, I wonder if this is a duplicate of #131474 ?

The problem is still present in the latest nightly, but maybe the fix for the above issue is not yet in nightly?

[avl]

Do you have a reproducer that we can look at to determine if the lint should or should not fire in this case? In any case, cc @Urgau as you know about non_local_definitions. I suspect this might be "working as intended" but it's hard to say without being able to look at an actual example.

Isn't the example in the bug-description enough? What more would you like to see?

[avl]

Here is an example in the rust playground: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=852a5e96a872b19ee42b26a0105275cf

[jieyouxu]

Ah wait I confused myself, I think this is #131474 my bad.

[jieyouxu]

I think this should have been addressed by #131498, but I'm not sure on the nightly timing

EDIT: no, this example is not covered by #131498

[avl]

I think this should have be addressed by #131498, but I'm not sure on the nightly timing, ~can you check if you are on latest nightly?~

Yeah, I think this whole bug report is a duplicate. Unless the fix from #131498 for some reason doesn't work in this particular case?

How can I determine if I have the fix or not?

[jieyouxu]

Let me double check the commits

[avl]

I'm using rustc 1.83.0-nightly (6b9676b45 2024-10-12) , which seems to be the latest nightly at the moment.

[jieyouxu]

The nightly commit is 6b9676b45 which is later than 7e05da8d42a730c15983bbd56dfbf744b32032d4 so I feel like the nightly should've already contained #131498.

[avl]

The nightly commit is 6b9676b which is later than 7e05da8 so I feel like the nightly should've already contained #131498.

Ok, then maybe #131498 doesn't solve this particular case. It's not entirely identical to #131474, which is what #131498 was to solve.

[jieyouxu]

I'll double-check the logic in the lint, I think this might not be the exact same thing yeah

[avl]

I tried rebuilding rustc from source, using latest 'master' (commit ef4e8259b50), and the problem exists on this version as well.

[jieyouxu]

My basic understanding is that unlike #131474

struct Test;
const _: () = {
    const _: () = {
        impl Default for Test {
            fn default() -> Test {
                Test
            }
        }
    };
};

the example in this issue

const _: () = {
    const _: () = {
        impl Callable for Dummy {}
    };
    pub trait Callable {}
    struct Dummy;
};

is different: the #131474 example produces a non-local definition inside nested anon consts for a outermost-level type Test outside of the anon consts. But Dummy in this example is not outermost-level which escapes the exception added by #131498

https://github.com/rust-lang/rust/blob/ecf2d1fa4bd8166c696883b10f483122b1fe98a3/compiler/rustc_lint/src/non_local_def.rs#L141-L170

[jieyouxu]

Remark: this would be fixed by #131660 once that is beta-backported.

[clarfonthey]

Also adding that yes, the original issue I mentioned in #131474 is still triggering because part of it is inside a proc macro. I thought it was because the nightly hadn't yet included the change but I checked and confirmed this myself.

[jieyouxu]

Also adding that yes, the original issue I mentioned in #131474 is still triggering because part of it is inside a proc macro. I thought it was because the nightly hadn't yet included the change but I checked and confirmed this myself.

To clarify, do you mean the fix for #131474 wasn't sufficient? Could you check if that's still a problem once #131660 is available in nightly, and if so open a new issue (or reopen if it's the same code pattern)?

[clarfonthey]

Yup, that's the plan: I just wanted to clarify that yes, the issue isn't fully resolved yet.