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

missing_debug_implementations does not lint on pin projected type #52

Closed
Kestrer opened this issue Jan 24, 2021 · 6 comments
Closed

missing_debug_implementations does not lint on pin projected type #52

Kestrer opened this issue Jan 24, 2021 · 6 comments
Labels
C-bug Category: related to a bug. C-upstream-bug Category: This is a bug of compiler or dependencies (the fix may require action in the upstream) S-blocked Status: Blocked on something else

Comments

@Kestrer
Copy link

Kestrer commented Jan 24, 2021

The following code compiles without warning or error:

#![forbid(missing_debug_implementations)]

pin_project_lite::pin_project! {
    pub struct X {
        inner: ()
    }
}

Would there be any way to prevent it from compiling? I have many futures and I would like to implement Debug on them, but currently I have to go through and check them all manually.

@taiki-e taiki-e added the C-bug Category: related to a bug. label Jan 25, 2021
@taiki-e
Copy link
Owner

taiki-e commented Jan 25, 2021

Thanks for the report.

I think this is due to the pin_project macro doesn't preserve the original visibility's span. In the following line, pub and struct's spans are replaced with call site span.

[$(#[$attrs])* pub struct $ident]

This should be able to fix by adding a step to separate visibility for parsing and visibility for code-generation.

FYI: I recently found the opposite behavior on pin-project and reported it to rustc as a bug. This comment on that issue explains why these problems occurs.

@taiki-e
Copy link
Owner

taiki-e commented Apr 24, 2021

I tried to fix this (4102899), but missing_debug_implementations uses the span of the whole struct (to be precise: the span of start and end), so to fix this problem we need to preserve the span of visibility and brace. This seems to be very difficult because there is no token type that can match a brace in declarative macros.

error: type does not implement `Debug`; consider adding `#[derive(Debug)]` or a manual implementation
   --> tests/test.rs:LL
    |
LL  | /     pub struct S {
LL  | |         f: (),
LL  | |     }
    | |_____^
    |

If you want to fix this problem, I think you will need to change the span used by missing_debug_implementations on the rustc side.

@taiki-e taiki-e added S-blocked Status: Blocked on something else C-upstream-bug Category: This is a bug of compiler or dependencies (the fix may require action in the upstream) help wanted Call for participation: Help is requested to fix this issue and removed help wanted Call for participation: Help is requested to fix this issue labels Apr 24, 2021
@Kestrer
Copy link
Author

Kestrer commented Apr 24, 2021

Well that's a shame :(. I don't see an easy way around it.

We could investigate the possibility of adding a small proc-macro dependency that doesn't use proc-macro2, quote or syn. It wouldn't affect build times too much; on my machine an empty proc macro crate increases it from 0.4s to 0.9s, where pin-project takes 15s. Another possible reason to do this would be that it could allow for the nicer syntax of an attribute. Although maybe this idea is best kept for a different crate.

@taiki-e
Copy link
Owner

taiki-e commented Apr 24, 2021

We could investigate the possibility of adding a small proc-macro dependency that doesn't use proc-macro2, quote or syn.

Well, I have one such crate, and am working on such a thing for another crate as well, so it is probably possible to look into it.
(Aside from whether I have the time to look it)

Another possible reason to do this would be that it could allow for the nicer syntax of an attribute.

Do you mean current #[project = ...] syntax?

Although maybe this idea is best kept for a different crate.

Yes.

on my machine an empty proc macro crate increases it from 0.4s to 0.9s, where pin-project takes 15s.

Btw, this is a clean build, right? Since the dependencies are only compiled the first time, I personally don't really understand why people who dislike proc-macro deps are always so concerned about the initial cost. (Also, even if we don't depend on proc-macro in the main build, we should often depend on it during development for testing and so on.)

@taiki-e
Copy link
Owner

taiki-e commented Apr 24, 2021

FWIW, if you just want to check if debug is implemented, it might be easier to create a codegen that generates something like assert_debug based on the codegen used in my project. (If you don't dislike writing proc-macro.)

@taiki-e
Copy link
Owner

taiki-e commented Jun 14, 2021

Closing (blocked-closed) -- It's impossible to fix this on the pin-project-lite side at this time. We can reopen the issue again once someone starts a discussion in rust-lang/rust about the span of missing_debug_implementations and the changes have been applied

@taiki-e taiki-e closed this as completed Jun 14, 2021
@taiki-e taiki-e removed the C-bug Category: related to a bug. label Jun 14, 2021
@taiki-e taiki-e added the C-bug Category: related to a bug. label Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: related to a bug. C-upstream-bug Category: This is a bug of compiler or dependencies (the fix may require action in the upstream) S-blocked Status: Blocked on something else
Projects
None yet
Development

No branches or pull requests

2 participants