search

salsa-rs / salsa

A generic framework for on-demand, incrementalized computation. Inspired by adapton, glimmer, and rustc's query system.
https://salsa-rs.netlify.app/
Apache License 2.0
2.12k stars 150 forks source link

How to avoid lints on generated code...? #487

Open nikomatsakis opened 6 months ago

nikomatsakis commented 6 months ago

483 improved the spans for field getters and things to use the span of the field. This makes working with salsa much nicer in rust-analyzer, generates better errors, etc, but it also leads to annoying clippy warnings in the generated code. I'm not 100% sure what's going on but my theory is that the newer spans are not tagged as coming from a procedural macro (they have no expansion stack) so clippy is not ignoring them anymore. Is there a "best practice" here that I am ignoring?

For now I added "allow" declarations in #486, but it doesn't seem right.

nikomatsakis commented 6 months ago

cc @xfrednet ?

xFrednet commented 6 months ago

but my theory is that the newer spans are not tagged as coming from a procedural macro

This is most likely the case. Clippy uses the spans from rustc and usually just checks if they are from an expansion. Span::from_expansion()

The quite_with_span macro prevents the expansion information to be added to the span. This can produce nicer error messages, but makes it difficult for Clippy.

Since this seems to generate an impl ... block, you can try to add the #[automatically_derived] attribute to it. That should suppress some lints.

It would be good if rustc could somehow retain the information where tokens come from, even if the span is manipulated by a macro, but currently that's not the case. The question would also be the question if that's actually worth it performence and memory wise.

Currently, we recomment to add #[allow] attributes like you did and #[automatically_derived]. One simple improvement could be to have rustc add the #[automatically_derived] attribute autoatically. It doesn't seem to do that always.

nikomatsakis commented 6 months ago

Thanks. I am not sure that rustc should always alter spans, but I wonder if there's an API I can use to do it myself. i.e., so I could mark the span as being from an expansion before I reuse it...

On Fri, Apr 5, 2024, at 8:33 AM, Fridtjof Stoldt wrote:

but my theory is that the newer spans are not tagged as coming from a procedural macro

This is most likely the case. Clippy uses the spans from rustc and usually just checks if they are from an expansion. Span::from_expansion() https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/span_encoding/struct.Span.html#method.from_expansion

The quite_with_span macro prevents the expansion information to be added to the span. This can produce nicer error messages, but makes it difficult for Clippy.

Since this seems to generate an impl ... block, you can try to add the #[automatically_derived] attribute to it. That should suppress some lints.

It would be good if rustc could somehow retain the information where tokens come from, even if the span is manipulated by a macro, but currently that's not the case. The question would also be the question if that's actually worth it performence and memory wise.

Currently, we recomment to add #[allow] attributes like you did and #[automatically_derived]. One simple improvement could be to have rustc add the #[automatically_derived] attribute autoatically. It doesn't seem to do that always.

— Reply to this email directly, view it on GitHub https://github.com/salsa-rs/salsa/issues/487#issuecomment-2039686579, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABF4ZT262RYPQEYLBO3UNTY32K2TAVCNFSM6AAAAABFY2YIQGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMZZGY4DMNJXHE. You are receiving this because you authored the thread.Message ID: @.***>