Allow and document clippy::drain_collect - #8500

No actionable comments were generated in the recent review. 🎉

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

@ShaharNaveh I went through the code in depth to check if the lint was necessary. It turns out that Clippy is overzealous here. The original code pre-allocates a buffer to reuse. The drain/collect idiom is intentional and correct. Instead, I documented the behavior and allowed the lint so that anyone else who encounters it knows not to fix the lint. I also fixed a totally unrelated but small lint. 😁

I used #[allow] instead of #[expect] because the lint doesn't trigger on my stable compiler but does trigger on nightly. CI would fail with #[expect].

@ShaharNaveh I went through the code in depth to check if the lint was necessary. It turns out that Clippy is overzealous here. The original code pre-allocates a buffer to reuse. The drain/collect idiom is intentional and correct. Instead, I documented the behavior and allowed the lint so that anyone else who encounters it knows not to fix the lint. I also fixed a totally unrelated but small lint. 😁

I used #[allow] instead of #[expect] because the lint doesn't trigger on my stable compiler but does trigger on nightly. CI would fail with #[expect].

tysm for looking into it!

@joshuamegnauth54 can we still add this rule? so future places would get flagged

I used #[allow] instead of #[expect] because the lint doesn't trigger on my stable compiler but does trigger on nightly. CI would fail with #[expect].

@coderabbitai Can #[expect] be used within #[cfg] so that #[expect] only applies when built with the nightly compiler?

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Rust toolchain and project policy files =='
fd -HI -t f '^(rust-toolchain(\.toml)?|Cargo\.toml|clippy\.toml|CONTRIBUTING\.md|README\.md)$' . \
  | sed -n '1,160p'

printf '%s\n' '== Compiler-version and nightly configuration references =='
rg -n -i -C 3 \
  '(rust-version|rustup|nightly|stable|rustversion|check-cfg|unexpected_cfgs|drain_collect)' \
  -g 'Cargo.toml' -g 'rust-toolchain*' -g 'clippy.toml' -g '*.rs' -g '*.md' . \
  | sed -n '1,280p'

printf '%s\n' '== Tail-call source map =='
ast-grep outline crates/vm/src/vm/mod.rs --items all --match 'trampoline|tail|call' --view expanded || true

printf '%s\n' '== Changed lines in relevant files =='
git diff -- crates/vm/src/vm/mod.rs crates/vm/src/stdlib/_codecs.rs

@fanninpm Yes, but Rust does not define a built-in cfg predicate for the compiler channel.

This attribute is valid if the build defines a custom nightly configuration option:

#[cfg_attr(nightly, expect(clippy::drain_collect))]
let refs: Vec<_> = refs.drain(..).collect();

A build script or CI must then define nightly only for nightly builds. A build script must also declare the option for check-cfg:

println!("cargo::rustc-check-cfg=cfg(nightly)");
println!("cargo::rustc-cfg=nightly");

The second line must run only when the selected rustc is nightly.

I do not recommend this for this PR. RustPython selects stable in rust-toolchain.toml and supports the latest stable Rust. The Clippy lint does not trigger consistently across those compiler versions. The documented #[allow(clippy::drain_collect)] therefore states the intended behavior without adding custom compiler-channel detection. #[expect] is best when the lint is expected to trigger on every supported toolchain.

_{You are interacting with an AI system.}

It turns out that Clippy is overzealous here. The original code pre-allocates a buffer to reuse. The drain/collect idiom is intentional and correct.

See rust-lang/rust-clippy#12410

@ShaharNaveh Do you mean enabling the lint or denying the lint? The lint should already be enabled by default, yet I'm unsure why it didn't trigger. Here's a Playground lint that demonstrates that it triggers on stable. 🤔

@ShaharNaveh Do you mean enabling the lint or denying the lint? The lint should already be enabled by default, yet I'm unsure why it didn't trigger. Here's a Playground lint that demonstrates that it triggers on stable. 🤔

oh, it didn't trigger due to clippy bug, nvm...

tysm!