We need a variable shadowing suppression option

Sigh... It's not the bugs we know of that are the problem. I have a 1.1M line C++ code base, and I can't PROVE there's any memory bugs in there. But I bet you there are.

If I have a compiler option that prevents me from doing any shadowing unless I specifically opt in, then I know damn well I won't have any shadowing related bugs because I probably won't ever opt in.

No one is going to hold a gun to your head and make you use such an option if it exists, and I don't need to justify my desire to have such an option to you. I'm the one who has to support my code, and I want the tools to help me do it the way I feel is safest.

1 Like

I have been following this thread, and I fail to understand why clippy doesn't suffice. It's simple, works now, and does exactly what you want. It can be used in an IDE (with rust-analyzer, at least), and is trivial to check in CI.

26 Likes

From a 5-year-old zombie thread that was just resurrected on URLO:

You can always just make it immutable afterward:

let mut s = String::new();
io::stdin().read_line(&mut s);
let s = s; // now immutable

This is one of the many ways that most experienced Rustaceans use shadowing. Many of these uses were not considered in the MISRA C spec simply because C does not offer the same compile-time checking as Rust.

22 Likes

The thing is that shadowing also helps prevent a different class of bugs, by virtue of making the original variable inaccessible. @Tom-Phinney gave one example; here is another. Consider this snippet that uses shadowing:

fn foo(a: &str) {
	let mut a: String = a.to_owned();
	a.push_str("foo");
	println!("{}", a);
}

And here is an equivalent without shadowing:

fn foo(a: &str) {
	let mut a2: String = a.to_owned();
	a2.push_str("foo");
	println!("{}", a);
}

Oops… I 'accidentally' wrote a instead of a2 in the last line. But the original a is still accessible, so it still compiles, resulting in incorrect behavior. That isn't possible with shadowing.

33 Likes

That said, if you truly want to forbid shadowing, it's indeed your choice: use Clippy. Your concerns about Clippy having poor IDE support are valid – well, I assume they are; I don't have experience with it myself. However, Clippy is widely recommended to Rust users, and if your goal is maximum safety, you should definitely be using Clippy for its other lints. Thus, the solution is not to move your favorite lint from Clippy to rustc, but to improve IDE support for Clippy so that everyone can take advantage of all of its lints.

18 Likes

This makes little sense here - if this were a large source of bugs in either new or experienced Rust programmers, we would expect to see a large number of people saying "oh yeah I was tripped up by this a couple of times". Instead we've just got you telling us how it's an issue in a different language.

This reads pretty badly - you absolutely have to justify it if you're asking a predominantly volunteer-based community to both implement and then maintain a feature that is questionably useful, and already implemented in the idiomatic tool. It sounds like what you're asking for is already implemented by the shadow_unrelated clippy lint, but even if it wasn't - what would prevent you from doing the work and implementing this lint if this is something you care about?

17 Likes

Not only that but stacking pinning (pin_utils::pin_mut) relies on shadowing for soundness.

11 Likes

And of course if you want to do it, do it. Even if you otherwise used the suppression you could opt in for specific scenarios if it really is useful. Even I might occasionally do it if there's some really good reason. But it want to be explicit, so that I know I'm never doing it by accident.

If you want to prohibit yourself from doing it without extra effort, that's your choice. Clippy already gives you a means of catching all such violations of your self-imposed policy. However, to insist on imposing your will on every other Rustacean, increasing their inconvenience, is not acceptable.

8 Likes

How exactly is having an option you don't remotely even have to use imposing on you? I'm failing to understand that. Are you going to be unable to prevent yourself from enabling it?

And of course the passive aggressive down-voters have shown up now and all my posts are just going to get hidden, so there's no point going on.

1 Like

There have been discussions about things like this in the past, and it's always been decided that Rust should not fracture into multiple "correct" dialects.

The Rust community generally accepts Clippy lints as being the idiomatic way of enforcing subsets of Rust on specific projects without needing to modify the compiler for each possible lint.

If you want to compile your project with a customized version of the rust compiler, you're welcome to do that, of course, but it's unlikely to be condoned.

4 Likes

Clippy works pretty well for me in vscode. I've got rust-analyzer installed, and in my settings.json, I set {"rust-analyzer.checkOnSave.command": "clippy"}. I get little yellow squigglies and everything. Can you explain (maybe a little less rudely than most of your comments so far) what you mean by "really bad."

26 Likes

They are being hidden because, frankly, you're being childish and disrespectful. As I said in my previous comment, you have the ability to do this now with clippy. Clippy can be used in an IDE — I use it on a daily basis in VS Code. I would be happy to provide you details on how to set it up if you are respectful.

Shadowing is idomstic Rust. However, if you want to go a different direction, feel free. Just enable the clippy lint and call it a day. There is zero reason to bring this lint into rustc, and you haven't made an argument for that beyond "I don't want to use clippy".

11 Likes

If you want the feature that badly, you can always add it yourself. Of course, since the rest of the community seems to think that it's a bad idea, it is unlikely to be merged back in to upstream, so you'll be stuck with perpetual maintenance.

You need to justify that because you are using it to try and convince others to perform work for you, for free. The least you can do then is be transparent about why, and even then it is only at their own discretion whether or not they decide to pick this up.

I personally am not particularly inclined to do so when the tone I'm being met with is somewhere between impatience and "just do what I say".

And that is of course ignoring that there already is a solution, clippy, which you seem to simply not want to use.

You either are trolling or clearly haven't been doing your homework. In less than 120 seconds I can verify this claim to be 100% false, simply by looking up the git history of clippy.

You still don't get it. You ask people to do things, you don't push them. If I were on such a team and read this, right now you'd have biased me against anything you have to say, due to the tone of it. It testifies of a lack of respect for the work others do that makes your life a lot better, and perhaps even a lack of respect for the people doing the work. I strongly suggest you do something about that.

17 Likes

You can also set RUSTC_WRAPPER=clippy-driver in your environment (or the equivalent in Cargo’s config.toml) if you want the linter to run automatically as part of every cargo build and cargo check command.

10 Likes

Wait, what? I (and others) was under the impression that clippy was named after Microsoft's Clippy assistant. If that wasn't the basis for clippy's name, what is?

1 Like

I think there's a confusion whether being "clearly based on MS Clippy" was an accusation of Rust clippy being literally based on Microsoft Office 97 codebase. It's of course absurd, so I don't know why this is even discussed.

Name of Rust's Clippy is a humorous reference to MS Clippy, but that has no bearing on what the tool does. It could have been called Modular Idiomatic Software Rust Analyzer.

21 Likes

Indeed I interpret "based on" as "either conceptually derived from, or inheriting code from".

In contrast, what's going on here is that clippy is named after the office assistant, and no further DNA is inherited from it.

Not having shadowing can lead to bugs, since it allows to accidentally use the variable that you wanted to shadow in later code.

It also means that you can't, in general, move code without changing variable names.

3 Likes

Why does the community keep engaging on this?

The OP's claims to have a 1.1 MLOC in C++ and based on that they raise concerns with Rust. This is not appropriate on an internals forum meant to discuss Rust's design.

As a professional C++ developer (day job... :man_shrugging: ) I'm frankly appalled by this attitude that is all too common within that community. OP did not display a desire to understand the rationale for Rust's design decisions nor did they provide examples in Rust for these so called issues.

As I said, this is exactly like a tourist to France that complains the street signs aren't in English. Rust is a welcoming community for anyone who genuinely wants to learn Rust but lets not feed the trolls that have no such intentions.

27 Likes