Skip to content

🐛 Adopt recoverable jeff deserialization errors - #2451

Merged
simon1hofmann merged 4 commits into
mainfrom
codex/2287-adopt-jeff-import-recovery
Sep 7, 2026
Merged

🐛 Adopt recoverable jeff deserialization errors#2451
simon1hofmann merged 4 commits into
mainfrom
codex/2287-adopt-jeff-import-recovery

Conversation

@simon1hofmann

@simon1hofmann simon1hofmann commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

🤖 AI text below 🤖

Description

Update jeff-mlir from 4732c4f to upstream commit 316eab7fab26d80a38278c8bf2ac2d249bda81a1, adopting the merged deserializer fix (#55) and Windows build fix (#56). The dependency uses the official unitaryfoundation/jeff-mlir repository.

Core already handles a null deserialization result. With the updated dependency, the previously fatal missing-functions input returns an import error instead of terminating the process. Add one in-process regression through JeffProgram::fromBytes using a serialized module without functions. Error handling stays upstream; no Core parser guards or exception-policy changes are added.

The deserializer requires exception support in KJ. The Windows fix enables /EHsc within the fetched Cap'n Proto dependency's CMake scope, including KJ and parser frames. The setting does not change Core's compiler flags.

Remove the unnecessary SetUp naming suppression. Its local warning came from incomplete SDK/header configuration; the override needs no source workaround.

Addresses the jeff import-abort finding in #2287. This PR is independent of the other audit fixes and is based on main at d52b3028c.

Validation

  • Current upstream-pin update: uvx nox -s lint passed. A clean CMake FetchContent download from the official repository checked out the exact merged commit.
  • The merged dependency tree matches the final Bump extern/dd_package from 3f1ea2e to 29caf80 #56 head exactly. Compared with the previously tested pin 650973f, only the Windows workflow/preset relocation differs; runtime sources, tests, and the scoped exception fix are unchanged.
  • At the previous Core revision eb654ca23, all 165 compiler tests and 151 jeff round-trip tests passed with LLVM/MLIR 23.1.0, including the missing-functions regression and successful imports. These tests were not rerun for the URL/tag-only update.
  • Full changed-file uvx nox -s cpp-lint passed at eb654ca23 with local clang-tidy 23.0.0git and the installed macOS SDK headers configured. No C++ files changed in the upstream-pin update.
  • Hosted CI at eb654ca23: Windows x64 and ARM64 C++ tests, Windows Python tests, and C++ lint with clang-tidy 23.1.1 passed. CI for the upstream-pin update remains pending.

GPT-6 via Codex assisted the dependency update, regression test, validation, and PR text.

Checklist

  • The pull request only contains commits that are focused and relevant to this change.
  • I have added appropriate tests that cover the new/changed functionality.
  • Documentation changes are not required: the public API is unchanged and the deserializer contract is documented upstream.
  • Changelog entries are not required for this unreleased v4 functionality.
  • Migration instructions are not required for this unreleased v4 functionality.
  • Local style and full changed-file C++ lint checks passed; hosted Windows tests and C++ lint also passed.
  • The changes are fully tested and pass the CI checks.
  • I have reviewed my own code changes.

If PR contains AI-assisted content:

  • Any agent that created, edited, or submitted GitHub content was explicitly authorized for that scope, as required by our AI Usage Guidelines.
  • Every agent-authored or agent-edited public text body begins with the visible disclosure 🤖 *AI text below* 🤖 (titles are exempt).
  • I have disclosed AI assistance in the PR description.
  • I confirm that I have personally reviewed and understood all AI-generated content, and accept full responsibility for it.

Update jeff-mlir to the merged upstream import-error fix and cover the former missing-functions abort through Core’s byte-import API.

Assisted-by: GPT-6 via Codex
@simon1hofmann simon1hofmann added bug Something isn't working dependencies Pull requests that update a dependency file MLIR Anything related to MLIR labels Sep 7, 2026
@simon1hofmann simon1hofmann self-assigned this Sep 7, 2026
@simon1hofmann simon1hofmann added this to the MLIR Support milestone Sep 7, 2026
@codecov

codecov Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@denialhaag denialhaag left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! :shipit:

I just have one observation:

Comment thread mlir/unittests/Compiler/test_compiler_pipeline.cpp Outdated
@simon1hofmann

Copy link
Copy Markdown
Contributor Author

@denialhaag Seems like the Windows failures are separate: jeff’s deserializer enables exceptions, but KJ builds without them, causing the missing kj::getCaughtExceptionAsKj() symbol. This needs an upstream build-configuration fix.

Will prepare one.

Use the upstream follow-up that enables exceptions for the fetched
Cap'n Proto dependency without changing Core's exception policy.
Remove the unnecessary SetUp naming suppression.

Assisted-by: GPT-6 via Codex
Restore the official repository now that jeff-mlir #56 has merged.

Assisted-by: GPT-6 via Codex
Assisted-by: GPT-6 via Codex
@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Important

Approval pending

CodeRabbit has no unresolved comments, but it has not reviewed the latest commit.

Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.

  • 🔍 Trigger 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.

❤️ Share

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

@simon1hofmann
simon1hofmann merged commit 2dc47f2 into main Sep 7, 2026
27 checks passed
@simon1hofmann
simon1hofmann deleted the codex/2287-adopt-jeff-import-recovery branch September 7, 2026 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working dependencies Pull requests that update a dependency file MLIR Anything related to MLIR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants