Repository navigation
Conversation
`ext_compile_args` passed the driver no optimization flag, so the fixed C shim (`src/ext/pycc_ext_module.c`) was compiled at -O0 even in a `--release` build. It now leads with `-O2` on every platform arm; every driver the build runs is GCC-compatible (system `cc`, or the bundled `clang` targeting MSVC on Windows), so one spelling serves all three. A build without `--release` keeps the ordinary profile semantics (debug `pycc_rt`, unoptimized IR); docs/RUNTIME.md and docs/CLI_SPEC.md now say so, and that ext timings come from `--release`, as the hosted ext protocol's Versions bullet already requires. docs/TESTING.md records the lark re-measurement: the release artifact goes from 2.63M to 2.44M instructions per parse (CPython 2.14M), 1.41x to 1.14x wall clock. Fixes #1516; part of #1514. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GLZC6kAwRGYgYdeXGRAjvM
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ 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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8bd224164
ℹ️ 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".
| **Build profile of an `ext` artifact ([#1516](https://github.com/rotnov/pycc/issues/1516)).** | ||
| An `--ext` build follows the ordinary `pycc build` profile rule | ||
| (`docs/CLI_SPEC.md`, `--release`): without `--release` (or a neighboring | ||
| `pycc.toml`'s `opt = "release"`) it emits unoptimized IR and links the | ||
| debug-profile `libpycc_rt.a`; with it, the O3-equivalent pipeline and the | ||
| release-profile runtime. The fixed C shim (`src/ext/pycc_ext_module.c`) is the | ||
| one exception: `ext_build::ext_compile_args` compiles it at `-O2` in every | ||
| profile, on every platform arm, since it is already-correct C on the path of |
There was a problem hiding this comment.
This establishes new build --ext behavior—optimizing the C shim in every profile while leaving IR and pycc_rt profile-dependent—but the commit does not update docs/ROADMAP.md's commit-relative delivery status. Record the new artifact/profile behavior and evidence there so the roadmap continues to describe the containing tree as required.
AGENTS.md reference: AGENTS.md:L38-L38
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in the next push: the CLI row of docs/ROADMAP.md now records that the shim is compiled at -O2 in every profile while IR and pycc_rt follow --release, with its evidence.
Generated by Claude Code
| **Re-measured after #1516 (2026-10-10; Part 2 of #1514).** The #1516 branch | ||
| compiles the C shim at `-O2` in every build profile, where `main` passed the | ||
| driver no optimization flag ("Build profile of an `ext` artifact" in | ||
| `docs/RUNTIME.md`). A build without `--release` still links the debug | ||
| `pycc_rt` and emits unoptimized IR, as any `pycc build` does. Timings of an |
There was a problem hiding this comment.
Once this paragraph records #1516 as delivered, the immediately preceding “remaining parts” list still describes #1516 as future work that will optimize both the shim and runtime in the default artifact. That conflicts directly with these added lines, which say the non-release artifact deliberately retains the debug runtime; rewrite the earlier list as historical or mark #1516 delivered with its actual scope.
AGENTS.md reference: AGENTS.md:L36-L36
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in the next push: the parts list is now introduced as the parts that remained after #1515, and the #1516 entry states the delivered scope (shim optimized in every profile; runtime and IR follow --release).
Generated by Claude Code
…1514 parts list Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GLZC6kAwRGYgYdeXGRAjvM
…ext-shim # Conflicts: # docs/TESTING.md
|
Generated by Claude Code |
Fixes #1516. Part of #1514, which stays open for #1517 and #1518.
Change
ext_build::ext_compile_argsnow leads with-O2(EXT_SHIM_OPT_LEVEL) on the Linux, macOS and Windows arms, so the fixed C shimsrc/ext/pycc_ext_module.cis compiled optimized in every build profile. Before this change the driver got no-Oflag, so the shim was built at-O0even under--release. One GNU spelling serves every arm because every driver the build runs is GCC-compatible: the systemccon Linux and macOS, and the bundledclangtargeting MSVC on Windows (linker_command).cl.exeis never invoked, so no/O2arm is needed.--releasekeeps ordinarypycc buildprofile semantics: unoptimized IR and the debugpycc_rt.docs/RUNTIME.mdhas a new "Build profile of anextartifact" paragraph, and the--extentry indocs/CLI_SPEC.mdnow says the same. The hosted ext protocol's Versions bullet indocs/TESTING.mdalready pins the pycc arm to--release, so the protocol needed no amendment. The new re-measure paragraph says that ext timings are taken from--releaseand that the debug rows are context only. This decision is reversible and project-local, so no ADR.-O2. A new test,every_platform_compiles_the_shim_at_o2_and_exactly_once, checks that every arm leads with the flag and carries exactly one-O//Oflag.Re-measurement (lark 1.3.1, subject
419d76a7..., CPython 3.14.7 uv build)The harness, machine and venvs are the same as for the #1515 re-measure. The artifact was built with
--ext --foreign-relative-importsby a debug build ofpycc. Five arms ran alternately, five processes each.parse(test_json)median (55 samples)main431a3366, no--release--releasemain431a3366,--release--releaseIr/parse is (Ir of 600 parses − Ir of 200 parses) / 400. CPython's per-process medians ranged from 214 to 275 µs, so the wall-clock ratios are noisier than the instruction counts. The parity driver matched CPython on the release artifact. The only difference was the order of set members in
str(e), which also varies between CPython runs.Verification
cargo fmt --checkandcargo clippy -p pycc --all-targets -- -D warningsare clean.cargo test --bin pycc -- ext --include-ignored: 352 passed.--include-ignored: 1036, 1048, 1049, 1050, 1054, 1066, 1067, 1112, 1143, 1145, 1199, 1397, 1427, 1453 and 1461.scripts/check_diff_coverage.py --require-changed-lines 100: 25 of 25 changed lines covered. This ran locally against acargo llvm-cov --bin pyccrun of the toolchain tests, not the full workspace run.deep-reviewer(D-068) is unavailable in this environment, so no local pinned review was run.🤖 Generated with Claude Code
https://claude.ai/code/session_01GLZC6kAwRGYgYdeXGRAjvM
Generated by Claude Code