Pre-RFC: Deprecation and separation of the dead_code lint

Summary

Deprecate the dead_code lint and give each item linted by dead_code its own item-specific unused lint as well as new lint that covers all unused items that acts as a subcategory of unused and contains the lints for unused items.

Motivation

In many cases, a user will want to silence a dead_code warning for a specific category of items, such as all imports while refactoring or dead variants of not dead functions. Since this is not possible, they will have to choose between allowing all things covered by dead_code or in that context or not silencing the warning.

For functions specifically, it is very rare to wish to silence all dead_code lints inside the function, most often it is the function itself that it is dead and silencing all dead_code lints on its interior is a negative side-effect.

The reason for deprecation and not giving dead code its own category, is that many, if not most programmers new to Rust but not programming in general, upon seeing a dead_code lint name, will assume it covers many, if not all, of the things in the unused category, especially so for the unreachable_code lint. The new unused_items lint should also cover its previous use cases while being clear.

There is also no apparent rhyme or reason as to why a certain items get their own unused lint right now or is grouped in with dead_code, i.e. why are imports not in dead_code? It makes it clear what kind of dead_code is expected in a given context when reading code.

Guide-level explanation

For existing Rust programmers: The dead_code lint is now deprecated, each of the different items it covered have their own lints now so if you have a dead struct you'd use an #[expect(unused_data_structures)] to expect that instead of dead_code. The exact list can be found here. If you wish to silence the warning for all items, use the unused_item lint.
(The dead code lint docs would be updated to have a table with each thing covered and what now covers that).

For new Rust programmers: Each type of unused code has its own lint, but they can be collectively toggled by the unused lint, the exact list of lints for unused code can be found here.

Reference-level explanation

The proposed list of items linted and the lint:

Modules......................................unused_modules
Functions....................................unused_functions
Type Aliases...............................unused_type_aliases
Structs, Enums, Unions..............unused_data_structures
Consts/Statics............................unused_compile_time_var
Traits..........................................unused_traits

And the lint for all items would be called unused_items and would include the following lints in its category: unused_modules, unused_functions, unused_type_aliases, unused_data_structures, unused_compile_time_var. unused_traits, unused_imports, (?) unused_macros.

During the transition period, dead_code would be changed to act as a subcategory of unused that contains these lints and emit a deprecation warning saying to replace it with either the individual lints or unused_items.

Drawbacks

More lint names for everyone to remember, though this can be mostly negated by adding the name to warning.

There is no way at this time to silence a warning on an attribute, so it would be impossible to silence the deprecation warning without silencing all deprecation warnings in the context. Given that the codebase would be a legacy codebase, it is likely that it'd already make sense to silence deprecations or there is little concern in doing so. There would be no other solution for this in versions of Rust with this deprecation, as an individual version of Rust can't get an update, that'd be a different version of Rust.

Rationale and alternatives

Deprecation rather than simply splitting dead_code and allowing people to choose between using the specific one or dead_code will ensure code readability by letting readers know what is dead and also reduces confusion for new rust users that expect it to include things like unreachable_code or unused_assignments.

Provide a way of applying a lint to a block/item itself rather than the item and everything inside of it.

This would also be useful outside of this but doesn't address most of the other motivations.

Use unused_type for type aliases, structs, enums and unions instead of unused_type_alias for the former and unused_data_structure for the latter 3.

Prior art

Thread about adding lint for constants specifically

Unresolved questions

  • What should the name of the lint for const and static variables be, given the currently proposed name is not very good? Or should they be separated instead?
  • Should macros be included in the unused_items lint? They act as an item in certain cases but not others.

Future possibilities

  • Allow applying a lint to a block itself without applying to all statements/items in the block.
5 Likes

There's some prior, related discussion in Create new lint for unused constants, #[allow(unused_constants)].

1 Like

Maybe dead_code should become a lint group instead, to make things like #![allow(dead_code)] still work (and not be deprecated) even though there will be more specific lints

1 Like

I originally was gonna suggest that, but then it becomes significantly harder for new users, and old users and dead_code is used more rarely, to remember what lints are under it vs unused. Naming the lints as dead_<item> doesn't solve this because what's covered by dead_code is relatively arbitrary. The docs say items, but imports aren't and in most languages dead_code is unreachable_code or any unused code so its much less intuitive and DWIM for newcomers.

This seems like a terrible user experience IMO. I would be strongly opposed to deprecating the existing dead_code. For one, because of churn. And for two, because it's extremely useful as a way to silence all dead code warnings. Which I often do during development. I don't want to have to go through and select the right lints to silence.

7 Likes

its an extremely useful way to silence all dead code warnings.

Its not though. It doesn't cover things like imports, macros, and assignments. Unused would be the correct lint here and the current name of dead_code and unused does a terrible job of communicating this. Adding an unused_items lint that covers all items, (the incorrectly stated affect of dead_code) would work better for this purpose.

TIL that this triggers unreachable_code, not dead_code. Interesting.

fn main() {
    std::process::exit(0);
    println!("dead code");
}

It seems worth adding an explanation to the RFC of what the dead_code lint does, since apparently it does not lint on dead code as in my example above.

unused is the general group for unused things, whatever they may be. The specific lints in that group do specific things (albeit maybe not all well named)

  • dead_code: lints on unused items
  • unreachable_code lints on unreachable code paths`
  • unreachable_patterns lints on match patterns that can never be reached

etc, etc

1 Like

*except imports for some reason

Given this definition, a lint rename from dead_code to unused_definitions or similar seems like a reasonable change to propose? And keep the old name around as a no longer recommended alias (like with bad_style and nonstandard_style).

(Specifying "definitions" instead of "items" I think captures the reasoning behind the lint not linting on unused imports, as those aren't item definitions.)

5 Likes

I mean, why not just change the documentation to say "unused definitions".

That's a fair point. I don't think unused_items covering all items was in the original proposal when I read it.

That addresses one of my concerns, and I think it is an improvement. But I think the churn caused by a deprecation here is not worth it. dead_code is very widely used. I don't think deprecation is necessary for improvement here though, to be clear.

I want to add that "dead code" is a common industry term that e.g. also finds its way into coding standards. "rustc can detect dead code by itself accurately" is a pretty big selling point and the feature having that name is really useful.

I also don't think it should be deprecated.

1 Like

The problem is, as shown by my example above, not all dead code that rustc detects triggers the dead code lint. So the dead_code rustc feature is not the same thing as "the rustc feature that detects dead code".

3 Likes

To me, the dead_code lint mostly does identify an interesting category, even if its name is odd. It’s the lint that tells you not just that you have superfluous things, but superfluous things you purposefully designed as meaningful API — not just unused lines of code, but unused function signatures. Unused imports are not interesting because you delete them and then if you need them, you just put them back; and unreachable code is most likely a simple logic bug or work in progress; but dead_code tells you something more big-picture about your application.

It wouldn’t necessarily be a good idea to fold unreachable_code into dead_code, for example, because allowing dead_code is something one might do, for example, when developing and testing functions before they're exported or used publicly in any way; in this situation, unreachable_code is still helping you see logic bugs.

Another nuance of the situation is that dead_code acts transitively (it reports all items not reachable even if they are mentioned by other unreachable items, and allowing it allows all the dependencies of the item it’s allowed for), and if (as the OP proposed) it were broken up into unused_functions and unused_compile_time_var, since both of those things can use each other, it would be less clear how the transitivity should work.

3 Likes

Maybe instead of replacing it with unused_items, replacing it with unused_api that acts identically to it does now would make more sense. As for transitivity, anything marked with an #[allow/expect(unused(_<thing>))] would be considered used in that context, as I believe it behaves now but will look into for things like imports. I'm going to add to the questions section whether the replacement should be called unused_items or unused_api and whether macros and/or imports should be included.

1 Like