Add OSS guard tests for ET_UNWRAP/ET_UNWRAP_TOKENIZER expression contract (#20075)#20075
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/20075
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (1 Unrelated Failure)As of commit 95fd0e9 with merge base db5f1ba ( FLAKY - The following job failed but was likely due to flakiness present on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@psiddh has exported this pull request. If you are a Meta employee, you can view the originating Diff in D107684052. |
This PR needs a
|
There was a problem hiding this comment.
Pull request overview
Adds OSS compile-time guard coverage to ensure ET_UNWRAP (in runtime/core/result.h) and ET_UNWRAP_TOKENIZER (in extension/llm/runner/util.h) continue to work as expression-form macros (usable as auto x = ET_UNWRAP(expr);) across both success and error paths—preventing regressions where they accidentally become statement-only macros that still compile in OSS but break downstream/internal consumers.
Changes:
- Add new
ET_UNWRAP(...)expression-position helpers and tests covering both the 1-arg and “with message” forms. - Add a new
ET_UNWRAP_TOKENIZER(...)expression-position helper and test that exercises success + error paths. - Add the required include in the LLM runner unit test to build the new guard.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| runtime/core/test/error_handling_test.cpp | Adds expression-position ET_UNWRAP helpers + gtests to pin the macro’s expression contract (including variadic “message” form). |
| extension/llm/runner/test/test_util.cpp | Adds expression-position ET_UNWRAP_TOKENIZER helper + gtest to pin the macro’s expression contract on prvalues. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ract (pytorch#20075) Summary: ET_UNWRAP (runtime/core/result.h) and ET_UNWRAP_TOKENIZER (extension/llm/runner/util.h) are expression-form macros used as `auto x = ET_UNWRAP(expr);` across 100+ call sites. No test anywhere exercised either macro, so changing them to statement-only macros (as pytorch#19686 did) compiled cleanly in OSS CI and only broke internal-only consumers (cria/arvr/jarvis) at diff-train import time. Add compile-time guard tests that use both macros in expression position (success + error paths) so OSS Buck and CMake CI fail if the expression contract is broken. Mirrored in fbcode + xplat. - error_handling_test.cpp: EtUnwrapExpressionForm, EtUnwrapExpressionFormWithMessage - test_util.cpp: EtUnwrapTokenizerExpressionForm Differential Revision: D107684052
|
@psiddh has imported this pull request. If you are a Meta employee, you can view this in D107684052. |
…ract (pytorch#20075) Summary: ET_UNWRAP (runtime/core/result.h) and ET_UNWRAP_TOKENIZER (extension/llm/runner/util.h) are expression-form macros used as `auto x = ET_UNWRAP(expr);` across 100+ call sites. No test anywhere exercised either macro, so changing them to statement-only macros (as pytorch#19686 did) compiled cleanly in OSS CI and only broke internal-only consumers (cria/arvr/jarvis) at diff-train import time. Add compile-time guard tests that use both macros in expression position (success + error paths) so OSS Buck and CMake CI fail if the expression contract is broken. Mirrored in fbcode + xplat. - error_handling_test.cpp: EtUnwrapExpressionForm, EtUnwrapExpressionFormWithMessage - test_util.cpp: EtUnwrapTokenizerExpressionForm Differential Revision: D107684052
f2487d4 to
95fd0e9
Compare
| Result<uint64_t> double_abs_unwrap(int64_t num) { | ||
| uint64_t value = ET_UNWRAP(get_abs(num)); | ||
| return value * 2; | ||
| } | ||
|
|
||
| Result<uint64_t> double_abs_unwrap_with_message(int64_t num) { | ||
| uint64_t value = | ||
| ET_UNWRAP(get_abs(num), "get_abs failed for %d", static_cast<int>(num)); | ||
| return value * 2; | ||
| } |
| ::executorch::runtime::Error | ||
| unwrap_tokenizer_expression(int32_t value, bool ok, int32_t& out) { | ||
| out = ET_UNWRAP_TOKENIZER(make_int_result(value, ok)); | ||
| return ::executorch::runtime::Error::Ok; | ||
| } |
Summary:
ET_UNWRAP (runtime/core/result.h) and ET_UNWRAP_TOKENIZER
(extension/llm/runner/util.h) are expression-form macros used as
auto x = ET_UNWRAP(expr);across 100+ call sites. No test anywhere exercisedeither macro, so changing them to statement-only macros (as #19686 did)
compiled cleanly in OSS CI and only broke internal-only consumers
(cria/arvr/jarvis) at diff-train import time.
Add compile-time guard tests that use both macros in expression position
(success + error paths) so OSS Buck and CMake CI fail if the expression
contract is broken. Mirrored in fbcode + xplat.
Differential Revision: D107684052