Skip to content

Fix MSVC build: replace ET_UNWRAP with manual error check#18798

Merged
meta-codesync[bot] merged 1 commit intomainfrom
export-D100218870
Apr 9, 2026
Merged

Fix MSVC build: replace ET_UNWRAP with manual error check#18798
meta-codesync[bot] merged 1 commit intomainfrom
export-D100218870

Conversation

@kirklandsign
Copy link
Copy Markdown
Contributor

Summary:
ET_UNWRAP uses GCC statement expressions ({...}) which MSVC does not
support, causing compilation failure on Windows CI.

Replace with equivalent manual check: get the Result, check ok(), return
error on failure, then dereference. Same semantics, cross-platform compatible.

Differential Revision: D100218870

Summary:
ET_UNWRAP uses GCC statement expressions ({...}) which MSVC does not
support, causing compilation failure on Windows CI.

Replace with equivalent manual check: get the Result, check ok(), return
error on failure, then dereference. Same semantics, cross-platform compatible.

Differential Revision: D100218870
Copilot AI review requested due to automatic review settings April 9, 2026 20:25
@pytorch-bot
Copy link
Copy Markdown

pytorch-bot Bot commented Apr 9, 2026

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/18798

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

⏳ 37 Pending, 1 Unrelated Failure

As of commit 2b8e8cd with merge base c5e917d (image):

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 9, 2026
@meta-codesync
Copy link
Copy Markdown
Contributor

meta-codesync Bot commented Apr 9, 2026

@kirklandsign has exported this pull request. If you are a Meta employee, you can view the originating Diff in D100218870.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 9, 2026

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes Windows/MSVC compilation by removing use of ET_UNWRAP (which relies on GCC/Clang statement-expressions) and replacing it with an equivalent, explicit Result check in the LLM runner metadata path.

Changes:

  • Replace ET_UNWRAP(module->get(...)) with auto get_result = ...; if (!get_result.ok()) return get_result.error(); to preserve error-propagation semantics on MSVC.
  • Keep metadata extraction behavior the same while making the implementation cross-platform.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@meta-codesync meta-codesync Bot merged commit 74aa25b into main Apr 9, 2026
208 of 216 checks passed
@meta-codesync meta-codesync Bot deleted the export-D100218870 branch April 9, 2026 22:29
jpiat pushed a commit to jpiat/executorch that referenced this pull request Apr 14, 2026
Differential Revision: D100218870

Pull Request resolved: pytorch#18798
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/cuda CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants