Variable shadowing, in my and many other people's opinions, is a bad thing. The compiler should have a no_shadow option so that it warns when this occurs. Variable shadowing is against Misra rules in C++ and I'd imagine it would be in Rust when/if that time comes. Given the stated applications for Rust, I'd think that being Misra complaint would be important. Given how easy it would be to warn of shadowing, I can see no reason not to provide that option, which could be switched off in such scenarios.
4 Likes
MISRA does not apply to Rust as-is. If MISRA-like rules for Rust are ever created and adopted, they would probably ship with a custom linter or compiler driver anyways that would enforce them, so it would be added there.
15 Likes
Also, could you clarify what kind of shadowing is forbidden by MISRA C++? Is it just locals shadowing members (which does not exist in Rust), or also locals in an inner block shadowing locals from an outer block?
2 Likes
You can do this via clippy, type "shadow" into https://rust-lang.github.io/rust-clippy/stable/index.html
Using shadowing is considered idiomatic, and so I would expect it to be extremely unlikely to be added to the main distribution.
35 Likes
Why make people use a third party tool to suppress something that clearly some to many people want to be able to suppress (else why would it be in the third party tool?) Given that it will likely be considered verboten for some to much critical software, why not just allow us to suppress it in the compiler? I mean the case of variables and constants and such are idiomatic as well, but they can be suppressed, and that's nothing but a stylistic choice.
Clippy can be installed with rustup. I wouldn't call it a third-party tool.
12 Likes
Clippy is a first-party tool, provided as part of the Rust ecosystem. That's why rustup supports it.
Like most things in Rust, clippy is separate from the rustc compiler and the std library so that semver can provide a way to make changes without breaking Rust's forward-compatability guarantees. std is the libary for things that require special compiler features (i.e., support); those things are permitted to change in sync with the compiler itself, and in fact are never permitted to be out of sync with the compiler..
(Admittedly there are some things in std that do not require special compiler support. They are there for historical reasons; if Rust 1.0 were to be released today most of them would be relocated to other libraries where semver would apply.)
11 Likes
Also, could you clarify what kind of shadowing is forbidden by MISRA C++? Is it just locals shadowing members (which does not exist in Rust), or also locals in an inner block shadowing locals from an outer block?
I haven't checked MISRA C++, but "locals in an inner block shadowing locals from an outer block" is definitely forbidden by MISRA C. It is MISRA C:2012 Required rule R.5.3. So I can't imagine it's different in MISRA C++.
2 Likes
That actually makes more sense as a restriction than the common Rust practice of successive shadowing redeclarations of the same variable in the same block as validation proceeds. That permits the same conceptual data to have a constant name as processing proceeds.
However, in Rust blocks are expressions, and as such could be needed during the course of those successive validation steps. Requiring the underlying data to have multiple names as it proceeds through the processing chain reduces the intelligibility of the code.
15 Likes
First or third, I meant more it's not built in. Coming from the C++ world of external linters, they pretty much suck compared to being part of the build. One of the nice (though sometimes annoying) things about Rust is that the compiler's doing it every time you build. I just tried loading clippy into VSC and that REALLY is annoying if that's the way it really works. I don't want that. I want output when I build and highlighting when I edit.
That actually makes more sense as a restriction than the common Rust practice of successive shadowing redeclarations of the same variable in the same block as validation proceeds.
As I understand, it is possible in Rust to shadow both in the same block and in different blocks. I think it may be a good idea to teach Clippy to know the difference. In fact, maybe I will implement this.
8 Likes
If you do implement such a restriction lint, please implement it in such a way that a block can be annotated to not restrict a specific identifier, without cancelling the restriction with respect to the rest of the module. That way a block expression that is needed within a processing chain where shadowing is intended is still usable. In the end such overriding of restriction becomes something like unsafe; more code inspection is required wherever such a restriction is overriden.
1 Like
Maybe we could pull this out and treat it as an independent feature request? I don't see any reason not to make it possible for an IDE to assimilate clippy just as thoroughly as it does rustc proper.
10 Likes
This isn't true, and is a misunderstanding of how compilers work. Shadowing isn't a special kind of mutation. To the compiler, it's exactly identical as if the new field had any other name.
18 Likes
In particular when the new shadowing variable has a different type, this pattern is usually fairly difficult to mess up. Perhaps a warning could apply when the variable isn't changing type.
5 Likes
There are substantially different cases involving shadowing, varying from totally idiomatic and harmless, to potential bugs, e.g.:
Same value transformed:
let a = something();
let a = a.as_ref();
Name repurposed:
let a = something();
let a = something_else();
The first case is totally idiomatic, useful, and I don't see any risks with it. The second case may be misleading, because it's not the same value any more.
Then there's a case of C-like shadowing without accessing the original value:
let a = 1;
{
let a = 2;
}
vs a case of accessing the original value:
let a = 1;
{
let a = 2;
}
add(a);
The first case is equivalent to just having a mutable variable, so it's relatively harmless. The second case may be a bug if the programmer didn't expect the value to "revert" to previous value.
So I think MISRA is about preventing errors likely in the last case, but Rust mostly makes use of the first case that is not even possible in C. Lints should be able to distinguish between them.
31 Likes
MISRA is an outdated collection of rules that go against current industry best practices.There were already investigations of auto manufacturers (e.g Toyota) where their MISRA compliant code caused fatalities. Experts testified how their code was a horrible mess of spaghetti code. Why would that be important or beneficial for Rust?
More over, applying best practices from another language blindly without evaluating the circumstances that brought them about is plain silly. Variable shadowing is indeed a bad practice in languages that are primarily procedural and mutable by default (such as C++). Rust however is descendent from Ocaml which is a functional style language in the ML family. Functional style languages (including Rust) which are immutable by default present a different set of trade-offs which makes shadowing beneficial and idiomatic.
This is akin to travelling to a foreign country and complaining about the locals' behaviour because it differs from the cultural norms of the visitor. If you are unwilling to learn idiomatic Rust and prefer instead idiomatic Misra style C++ code than why come and post here?
26 Likes
There's also the use case of cloning an Rc or Arc several times to move it into a closure. Since all clones point to the same value, shadowing the variable name has no risk of introducing bugs.
4 Likes
It has nothing to do with style or with C++. It has to do with human falliblity and the fact that if the compiler doesn't tell you've done something that could be a bug if unintentional, and shadowing could very well be such potentially, then that's just asking for problems in large code bases that must be maintained by changing staff over many years. Your customers don't care if you are hip or idiomatic, they care about safe and reliable product.
Instead of worrying about it, the safe thing is to just make it an opt in and then you know it's only done intentionally. You don't have to do that, but it needs to be an option for those folks who care about such safety issues. If it's my heart attached to the machine, I could give a damn about the convenience of the developer or whether he was feeling unidiomatic at the time.
Do you have any examples of real bugs in Rust code due to variable shadowing?
12 Likes