-
Notifications
You must be signed in to change notification settings - Fork 0
fix(pycc_types): reject unrenderable instance and protocol string conversion with C0001 (#977) - #985
fix(pycc_types): reject unrenderable instance and protocol string conversion with C0001 (#977) #985rotnov merged 16 commits into
Conversation
...nversion with C0001 (#977) `print(<instance>)` and `f"{<instance>}"` of a class instance that neither MIR rewrite renders passed `pycc check` and panicked in `pycc build`, and several builtin-exception-name shadowing shapes passed `check` and aborted at runtime. A new `pycc_types::string_conversion` predicate, called at the f-string interpolation and `print` argument sites, now rejects every such shape with `C0001`: the 25 builtin exception names are decided by provenance (seeded synthetic class, or unseeded flat seven), every other name by dataclass shape, and a protocol-typed value with its own message. Comment-only corrections in `pycc_mir`/`pycc_codegen` state the real gate. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
...ecs (#977) Add ADR D-237 (predicate table, the two divergences from MIR's name-first resolution, alternatives, the newly rejected dataclass-under-exception-name shape), regenerate the decisions index, amend the dataclass rendering sentence in TYPE_SYSTEM.md and ROADMAP.md, qualify the 2026年08月01日 tuple follow-up clause, add the v0.4 #977 row, describe the pycc_types C0001 emission and both message shapes in DIAGNOSTICS.md and the C0001 explain prose, write session snapshot 2026年09月06日-13, and add the PR #971 eighteen-round retrospective entry. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
... close a helper coverage gap (#977) Review round 1: add public-CLI rejections for a non-dataclass subclass of a dataclass, a plain PEP 695 generic instance, and `print(self)` inside a method, so every row of D-237's repro table is pinned; replace the `assert_accepted` helper's never-taken panic branch with `expect` so the 100% region gate holds. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
i️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f6784166a
i️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
...zation; rename help for builtin-named classes Codex round 1 on PR #985. P1: `instantiate_generic_class_methods` built every specialization with `is_dataclass: false` and an empty `dataclass_fields`, so `check` accepted `print(Box[int](1))` against the origin `@dataclass Box` while `build` re-inferred the rewritten call against `0gen_Box__T_int` and the D-237 gate rejected it (before the gate, MIR skipped the `__repr__` rewrite for the same reason and codegen panicked). The specialization now carries the origin's `is_dataclass` and its `dataclass_fields` substituted like `attrs`; `methods` already carried the mangled synthesized `__repr__`, so the specialization renders as `Box(n=1)`. P2: the help recommended `@dataclass` for a class under a builtin exception name, which the predicate rejects by name regardless; such a class now gets a rename remedy instead. Tests: four new unit tests (help variants, specialization metadata via `check_and_resolve` on both a parsed and a hand-built module) and two public-CLI tests (generic dataclass check/run consistency; rename help via `--error-format json` on a `print(self)` shape, since a call to the shadowing class is reported by the solver's own message under D-220). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
... PR #985) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd21b24f73
i️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
...rsion sites Codex round 2 on PR #985. `pycc_mir` erases `cast(Base, d)` to `d` and its `__repr__` rewrite keys on the erased value's class, so an upcast from a plain subclass to a `@dataclass` base passed the type-only gate as `Base` and panicked in codegen's `to_str` as a `Derived`. The new `reject_unrenderable_expr` gates the expression's type and then the value under every erased `cast` (nested included; a user `def cast` is not looked through), at both the `print` and f-string sites. Tests: five unit tests (print, f-string, nested cast, cast to the dataclass's own type accepted, user `def cast` not looked through) and a public-CLI rejection for the reported shape. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
...ound 2 and the bound-cast follow-up Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
... string-conversion descent `check_cast` has already inferred the same value successfully by the time `reject_unrenderable_expr` descends into an erased `cast`, so the `?` on that second inference was an unreachable branch and left one uncovered region under the D-014 100%/100% gate. State the invariant with `expect` instead. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
... merging main `origin/main` gained #986, which pushed `docs/ROADMAP.md` past the 168960-byte per-resource llms.txt budget `scripts/check-site.sh` enforces once this branch's own #977 paragraph merged alongside it. Condense the #977 landing paragraph and the sentence duplicating it inside the #378 paragraph, and re-anchor the session file's resume steps on the merged state. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
...991) The `Pages` workflow's `Validate website` step failed on `main`: `scripts/check_sitemap_lastmod.rb` requires the sitemap `<lastmod>` for `https://rotnov.github.io/pycc/status/` to equal the author date of the last non-merge commit touching `site/status/index.html`. PR #985 stamped `2026年09月06日`, but its squash merge (`3ba4a027`) landed on `2026年09月07日`, so `deploy` and `notify-indexnow` were skipped and the site stopped publishing. Rotate the four status-page pins to `2026年09月07日`, byte-neutrally: the `/status/` sitemap `<lastmod>`, the page's JSON-LD `dateModified`, `check-site.sh`'s `PAGE_SPECS["status"]["date_modified"]`, and the status entry's `source_artifact_sha256` in the pages-performance manifest. The landing page's own assertion and the other six pages' pins stay at `2026年09月06日`, matching their source files' commit dates. The checker is deliberately not relaxed here. The proposed `>=` relaxation does not make this failure pass — the stale stamp is *earlier* than the commit date — and accepting an earlier stamp would permit exactly the staleness the checker exists to catch. The real cause is that `--no-merges` assumes a merge-commit topology that squash-merge defeats; that is tracked as #990. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Summary
Fixes #977.
pycc checkacceptedprint(instance)andf"{instance}"for any class instance, and every non-dataclass instance then panicked inpycc_codegen'sto_str(lib.rs:1593). This change adds a string-conversion gate inpycc_types(string_conversion.rs, D-237) that rejects withC0001, before lowering, everyprintargument and f-string interpolation the backend cannot render:__init__, with or without a user__repr__), enum members, generic instances,self, and non-dataclass subclasses of a dataclass;@dataclass, and tagged user exception classes instantiated as values (MIR's name-first exception rewrite would otherwise render aPyInstanceObjas an exception object).@dataclassinstances and caught builtin exceptions keep rendering exactly as before. The one deliberate loss is a@dataclassdeclared under a builtin exception name, which rendered by accident through the exception rewrite; D-237 records the rationale. The positive path (calling a user__repr__) is deferred: it needs D-236's reserved-name machinery and is out of scope here.Changes
crates/pycc_types/src/string_conversion.rs:StringConversionSite+reject_unrenderable, a provenance-based predicate, and unit tests covering every predicate region.crates/pycc_types/src/expr.rs: the f-string interpolation arm and the singleprintargument funnel call the gate.tests/issue_977_instance_string_conversion.rs: public-CLI tests pinning every crash shape from the plan's repro table plus accept controls.pycc_codegen,pycc_mir, andpycc_types/exception.rscorrecting the claim that the checker already rejected these shapes.docs/TYPE_SYSTEM.md,docs/DIAGNOSTICS.md(C0001 provenance),docs/ROADMAP.md(printrestriction line and the Printing an instance of a class without__repr__passescheckand panics inpycc_codegen(lib.rs:1593) #977 evidence row),explain.rsC0001 prose, session filedocs/sessions/2026-09-06-13-..., and a D-066 retrospective entry about PR fix(hir): report the enum-call C0001 at the call expression (#944) #971 's eighteen review rounds..harden/findings/issue-977.jsonl: the D-068 review round (one test-coverage gap fixed, one methodology note refuted).Verification
Local gate set on the rebased head (
e77b4b13base): fmt, clippy-D warnings,cargo test --workspace,cargo llvm-cov --workspace --fail-under-lines 100 --fail-under-regions 100(TOTAL 100.00% / 100.00%),cargo doc, decisions-index--check, roadmap-evidence checker,scripts/unittests, agent asset/policy validators,check_ci_permissions.rb— all exit 0.Adjacent observation, not filed (D-192 non-milestone ceiling in force): rendering a caught exception twice panics in
pycc_rt(lib.rs:934); recorded in the session file as a follow-up.