interpreter: suppress the module-miss __spec__ diagnosis on swallowed attribute lookups

[fregataa]

Problem

python3 pyre/check.py's synth/import_from_hot ran ~100x slower than pypy. Profiling showed the compiled loop's residual IMPORT_NAME call spending most of its time constructing an AttributeError that is immediately discarded: dunder_import's fast path probes the module for __path__ via findattr_result, and every miss ran the full __spec__ shadowing diagnosis — including the has_location property getter, which executes a Python frame — just to phrase a message nobody sees.

Fix

Mirror CPython's _PyObject_LookupAttr / module_getattro_impl(suppress) semantics: thread a suppress flag through getattr_str_impl → module_getattr_fallback → module_getattr_hook_or_err. Suppressed lookups still run the full protocol — the PEP 562 module __getattr__ hook is called as before — but a terminal module miss keeps the plain error instead of building the diagnosis.

Suppressed callers (all of which swallow the AttributeError, matching CPython's use of the suppressed lookup at each site):

• findattr / findattr_result (covers dunder_import's __path__ probe)
• builtin_hasattr
• builtin_getattr when a default is supplied

Surfaced misses (math.nope, explicit ModuleType.__getattribute__, from-import failures) are unchanged; verified message parity against CPython, including the PEP 562 hook paths.

Results

• synth/import_from_hot: 0.57s → 0.23s wall; vs pypy ~100x → 40.0x (dynasm) / 59.6x (cranelift); gate tightened max-pypy-ratio=764 → 180
• Full pyre/check.py: ALL PASSED — dynasm 371/371, cranelift 371/371

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Improved hasattr so missing attributes return False without triggering unnecessary module diagnostics.
  • Improved getattr with a default value while preserving normal error reporting when no default is provided.
  • Refined attribute lookup for descriptors, iterators, and length hints to handle suppressed lookup consistently.

• Chores

  • Updated benchmark configuration and clarified related diagnostic guidance.

[coderabbitai]

Warning

Review limit reached

@youknowone, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 24 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 706dc31e-f193-496d-a944-96d744ae5027

📥 Commits

Reviewing files that changed from the base of the PR and between cc591e7 and a013993.

📒 Files selected for processing (3)

• pyre/bench/synth/import_from_hot.py
• pyre/pyre-interpreter/src/baseobjspace.rs
• pyre/pyre-interpreter/src/builtins.rs

Walkthrough

The interpreter adds suppressed attribute lookup through shared lookup and module fallback paths. hasattr and defaulted getattr use the new path. The benchmark configuration lowers the Pyre ratio limit and documents suppressed lookup behavior.

Changes

Attribute lookup behavior

Layer / File(s)	Summary
Lookup suppression propagation pyre/pyre-interpreter/src/baseobjspace.rs	Shared lookup, descriptor handling, module fallback, iterator detection, and length-hint lookup now pass an explicit suppression flag. Suppressed module misses return the original AttributeError before __spec__ diagnosis.
Public lookup API and callers pyre/pyre-interpreter/src/baseobjspace.rs, pyre/pyre-interpreter/src/builtins.rs	lookup_attr validates names and performs suppressed lookup. hasattr and defaulted getattr use this path, while ordinary getattr remains raising.
Benchmark configuration pyre/bench/synth/import_from_hot.py	The Pyre ratio limit changes from 764 to 180. Comments document __path__ probe suppression and skipped __spec__ shadowing diagnosis.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

• youknowone/RustPython#48: The change adds the non-raising attribute lookup path described by the issue.

Possibly related PRs

• youknowone/pyre#731: Adds related module metadata handling for __spec__ and other import attributes.
• youknowone/pyre#798: Modifies related fallible attribute lookup behavior in baseobjspace.rs.
• youknowone/pyre#848: Modifies related __getattr__ fallback and AttributeError handling.

Suggested reviewers: youknowone, lifthrasiir

Poem

A rabbit checks each hidden name,
While modules skip the shadow game.
hasattr hops through quiet ways,
Defaulted getattr finds its place.
The ratio drops; the paths grow clear.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change to suppress module-miss spec diagnosis during swallowed attribute lookups.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[youknowone]

@fregataa it makes sense, thank you! other failures seems irrelevant to this change, but could you check about the cargo test failure?

[fregataa]

Looked into the cargo test failure. It was the pyre-object lib-test binary aborting on ubuntu with memory allocation of 8 bytes failed (SIGABRT) partway through its 308 tests, with no single test attributable (parallel harness, the aborting test never printed its name).

I believe it's a one-off runner OOM rather than something from this PR:

• This PR only touches pyre-interpreter (plus a bench header); pyre-object is unchanged, and the -p pyre-object --lib test binary doesn't link pyre-interpreter at all.
• The same job passes on main (including runs newer than this PR's base), on other recent PRs, and on macOS/Windows within this very run.
• cargo test -p pyre-object --lib passes locally on this branch.

I've rebased onto current main and repushed, so CI is running again — will report if the failure reproduces.

🤖 Generated with Claude Code

[youknowone]

Thank you for confirming!