Opcode descs not hardcoded - #7682

crates/compiler-core/src/bytecode/oparg.rs (3)

395-414: desc() signature: take self and return &'static str.

Two minor concerns shared by all three new desc() impls:

• The receiver is &self, but each of these enums is Copy and the sibling methods generated by the macro (as_u8(self), as_u32(self)) and BinaryOperator::as_inplace(self) take self. Take self for consistency.
• The return type &str ties the returned slice's lifetime to &self via elision. Since every arm returns a string literal, declaring &'static str is more accurate and frees callers from an unnecessary borrow on the receiver.

♻️ Suggested signature change (apply to all three impls)

-    pub const fn desc(&self) -> &str {
+    pub const fn desc(self) -> &'static str {
         match self {

Also applies to: 430-443, 673-706

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/compiler-core/src/bytecode/oparg.rs` around lines 395 - 414, The
desc() impls currently use &self -> &str which unnecessarily ties the returned
string's lifetime to the receiver and is inconsistent with other methods; change
the signature of IntrinsicFunction1::desc (and the two other similar impls) to
take self (by value) and return &'static str, i.e. pub const fn desc(self) ->
&'static str, and keep the match arms returning the same string literals so
callers don't need a borrow and lifetimes are correct.

---

246-250: Prefer into_iter() over iter().copied().

In Rust 2021 edition, arrays implement IntoIterator yielding owned values, which is the idiomatic and slightly cheaper way to enumerate Copy enum variants. Also consider renaming to iter() or variants() to match Rust naming conventions — iterator() is uncommon.

♻️ Suggested change

             /// Iterate over the variants.
             #[must_use]
-            $vis fn iterator() -> impl Iterator<Item = Self> {
-                [$(Self::$variant),*].iter().copied()
+            $vis fn iter() -> impl Iterator<Item = Self> {
+                [$(Self::$variant),*].into_iter()
             }

Note: callers in crates/stdlib/src/_opcode.rs (IntrinsicFunction1::iterator(), etc.) would need to be updated accordingly.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/compiler-core/src/bytecode/oparg.rs` around lines 246 - 250, The
current iterator() returns owned enum variants via [.iter().copied()], which is
non-idiomatic; change the method (e.g., rename iterator() to iter() or
variants()) to return the iterator using the array's IntoIterator (use [. .
.].into_iter()) and update all callers (e.g., IntrinsicFunction1::iterator()
usages in crates/stdlib/src/_opcode.rs) to the new name; ensure the signature
and visibility ($vis fn ...) remain the same except for the name and that the
body uses into_iter() instead of iter().copied().

---

396-396: Use a markdown link for the CPython source reference.

Bare URLs in doc comments are flagged by clippy::doc_markdown and are inconsistent with the existing convention in this file (e.g. line 908 uses [CPython ...](https://...)). Wrap them in a markdown link or angle brackets.

♻️ Example for line 396 (apply analogously at lines 431 and 673)

-    /// https://github.com/python/cpython/blob/v3.14.4/Include/internal/pycore_intrinsics.h#L9-L20
+    /// See [CPython `pycore_intrinsics.h`](https://github.com/python/cpython/blob/v3.14.4/Include/internal/pycore_intrinsics.h#L9-L20).

As per coding guidelines: "Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints that are introduced by your changes".

Also applies to: 431-431, 673-673

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/compiler-core/src/bytecode/oparg.rs` at line 396, Doc comments in
crates/compiler-core/src/bytecode/oparg.rs contain bare URLs (e.g., the CPython
reference at the comment currently shown) which triggers clippy::doc_markdown;
update those doc comments to use markdown links or angle-bracketed links instead
of plain URLs. Locate the three offending doc comments in oparg.rs (the CPython
reference occurrences around the noted spots) and replace each bare URL with a
Markdown link like [CPython
pycore_intrinsics.h](https://github.com/python/cpython/blob/v3.14.4/Include/internal/pycore_intrinsics.h#L9-L20)
or use <https://...> so clippy no longer flags them, then run cargo clippy and
fix any resultant doc_markdown warnings.

crates/compiler-core/src/bytecode/oparg.rs (3)

394-413: Consider taking self by value and returning &'static str from desc.

IntrinsicFunction1, IntrinsicFunction2, and BinaryOperator are all Copy enums, and every arm of desc returns a string literal whose true lifetime is 'static. The current &self signatures elide the return lifetime to &'a str tied to the receiver, which forces callers to keep the receiver alive even though the data is static, and is inconsistent with the existing by-value as_inplace(self) API on BinaryOperator. Returning &'static str is also nicer for callers like _opcode.rs that pass the result straight into vm.ctx.new_str(...).

♻️ Suggested signature (apply analogously to all three impls)

-    /// https://github.com/python/cpython/blob/v3.14.4/Include/internal/pycore_intrinsics.h#L9-L20
-    #[must_use]
-    pub const fn desc(&self) -> &str {
+    /// See [CPython `pycore_intrinsics.h`](https://github.com/python/cpython/blob/v3.14.4/Include/internal/pycore_intrinsics.h#L9-L20).
+    #[must_use]
+    pub const fn desc(self) -> &'static str {
         match self {
             Self::Invalid => "INTRINSIC_1_INVALID",
             // ...
         }
     }

Also applies to: 429-442, 671-705

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/compiler-core/src/bytecode/oparg.rs` around lines 394 - 413, The desc
methods currently take &self and return an elided &str tied to the receiver;
change them to take self by value and return &'static str (e.g. pub const fn
desc(self) -> &'static str) because IntrinsicFunction1, IntrinsicFunction2 and
BinaryOperator are Copy enums and all arms return string literals; update the
three impls (IntrinsicFunction1::desc, IntrinsicFunction2::desc,
BinaryOperator::desc) accordingly and adjust any callers that relied on a
borrowed receiver to pass the enum by value instead (callers like _opcode.rs
that pass straight into vm.ctx.new_str(...) will continue to work and benefit
from a 'static return).

---

395-395: Wrap bare URLs in doc comments.

These three doc comments use plain URLs as their entire content. Rustdoc's bare_urls lint flags this, and the rest of this file consistently uses Markdown links (e.g., [CPython RESUME_OPARG_LOCATION_MASK](...) near ResumeContext). For consistency and to avoid potential lint failures under -D warnings, prefer <https://...> or a Markdown link.

♻️ Example fix

-    /// https://github.com/python/cpython/blob/v3.14.4/Include/internal/pycore_intrinsics.h#L9-L20
+    /// See [CPython `pycore_intrinsics.h` (intrinsic 1 table)](https://github.com/python/cpython/blob/v3.14.4/Include/internal/pycore_intrinsics.h#L9-L20).

As per coding guidelines: "Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints that are introduced by your changes".

Also applies to: 430-430, 672-672

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/compiler-core/src/bytecode/oparg.rs` at line 395, Several doc comments
in crates/compiler-core/src/bytecode/oparg.rs contain bare URLs (e.g., the
CPython pycore_intrinsics link and the comment near ResumeContext) which
triggers the Rustdoc `bare_urls` lint; update each such doc comment to wrap the
URL in angle brackets like <https://...> or convert it to a Markdown link
`[...](https://...)` so the lint is satisfied and formatting matches the rest of
the file. Ensure you modify all three occurrences referenced in the review (the
pycore_intrinsics link and the two other bare-URL comments) so clippy/cargo
clippy no longer reports warnings.

---

246-249: Use .into_iter() instead of .iter().copied() for better idiom and clarity.

All enums generated by this macro have Copy derived, making .into_iter() applicable to all uses. This change is more idiomatic in Rust 2021+, avoids relying on array literal promotion semantics, and automatically preserves ExactSizeIterator and DoubleEndedIterator traits without additional reasoning.

♻️ Proposed change

             /// Iterate over the variants.
             $vis fn iter() -> impl Iterator<Item = Self> {
-                [$(Self::$variant),*].iter().copied()
+                [$(Self::$variant),*].into_iter()
             }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/compiler-core/src/bytecode/oparg.rs` around lines 246 - 249, The
generated enum iterator function ($vis fn iter() -> impl Iterator<Item = Self>)
should use array into_iter() instead of calling .iter().copied(); locate the
macro output that builds the array of variants ([ $(Self::$variant),* ]) and
replace the trailing .iter().copied() call with .into_iter() so the iterator is
produced idiomatically (preserves ExactSizeIterator/DoubleEndedIterator for the
Copy enum variants).