Skip to content

Commit fb09045

Browse files
psiddhfacebook-github-bot
authored andcommitted
Add OSS guard tests for ET_UNWRAP/ET_UNWRAP_TOKENIZER expression contract (#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 #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
1 parent 12684ef commit fb09045

2 files changed

Lines changed: 66 additions & 0 deletions

File tree

extension/llm/runner/test/test_util.cpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88

99
#include <executorch/extension/llm/runner/util.h>
10+
#include <executorch/runtime/core/result.h>
1011
#include <executorch/runtime/platform/runtime.h>
1112

1213
#include <gtest/gtest.h>
@@ -63,4 +64,32 @@ TEST_F(ConvertToBFloat16Test, RejectsNonFloatTensor) {
6364
EXPECT_EQ(result.error(), ::executorch::runtime::Error::InvalidArgument);
6465
}
6566

67+
::executorch::runtime::Result<int32_t> make_int_result(int32_t value, bool ok) {
68+
if (ok) {
69+
return value;
70+
}
71+
return ::executorch::runtime::Error::InvalidArgument;
72+
}
73+
74+
// Uses ET_UNWRAP_TOKENIZER in expression position (on a prvalue, like real call
75+
// sites) to pin its expression-form contract (see util.h): a change requiring a
76+
// variable-name argument would stop this from compiling.
77+
::executorch::runtime::Error
78+
unwrap_tokenizer_expression(int32_t value, bool ok, int32_t& out) {
79+
out = ET_UNWRAP_TOKENIZER(make_int_result(value, ok));
80+
return ::executorch::runtime::Error::Ok;
81+
}
82+
83+
TEST_F(ConvertToBFloat16Test, EtUnwrapTokenizerExpressionForm) {
84+
int32_t out = -1;
85+
auto ok_err = unwrap_tokenizer_expression(7, /*ok=*/true, out);
86+
EXPECT_EQ(ok_err, ::executorch::runtime::Error::Ok);
87+
EXPECT_EQ(out, 7);
88+
89+
out = -1;
90+
auto bad_err = unwrap_tokenizer_expression(9, /*ok=*/false, out);
91+
EXPECT_EQ(bad_err, ::executorch::runtime::Error::InvalidArgument);
92+
EXPECT_EQ(out, -1);
93+
}
94+
6695
} // namespace

runtime/core/test/error_handling_test.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,21 @@ Result<uint64_t> get_abs(int64_t num) {
2828
}
2929
}
3030

31+
// Helpers that use ET_UNWRAP in expression position. They pin the macro's
32+
// expression-form contract (see result.h): a change that turns ET_UNWRAP back
33+
// into a statement-only macro (requiring a variable-name argument) would stop
34+
// these from compiling.
35+
Result<uint64_t> double_abs_unwrap(int64_t num) {
36+
uint64_t value = ET_UNWRAP(get_abs(num));
37+
return value * 2;
38+
}
39+
40+
Result<uint64_t> double_abs_unwrap_with_message(int64_t num) {
41+
uint64_t value =
42+
ET_UNWRAP(get_abs(num), "get_abs failed for %d", static_cast<int>(num));
43+
return value * 2;
44+
}
45+
3146
Result<std::string> get_op_name(int64_t op) {
3247
auto abs_result = get_abs(op);
3348
if (!abs_result.ok()) {
@@ -243,6 +258,28 @@ TEST(ErrorHandlingTest, ResultUnwrap) {
243258
ASSERT_EQ(res.error(), Error::InvalidArgument);
244259
}
245260

261+
TEST(ErrorHandlingTest, EtUnwrapExpressionForm) {
262+
auto ok = double_abs_unwrap(5);
263+
ASSERT_TRUE(ok.ok());
264+
EXPECT_EQ(*ok, 10u);
265+
266+
auto err = double_abs_unwrap(-1);
267+
EXPECT_FALSE(err.ok());
268+
EXPECT_EQ(err.error(), Error::InvalidArgument);
269+
}
270+
271+
TEST(ErrorHandlingTest, EtUnwrapExpressionFormWithMessage) {
272+
executorch::runtime::runtime_init();
273+
274+
auto ok = double_abs_unwrap_with_message(4);
275+
ASSERT_TRUE(ok.ok());
276+
EXPECT_EQ(*ok, 8u);
277+
278+
auto err = double_abs_unwrap_with_message(-2);
279+
EXPECT_FALSE(err.ok());
280+
EXPECT_EQ(err.error(), Error::InvalidArgument);
281+
}
282+
246283
TEST(ErrorHandlingTest, ResultNoCopy) {
247284
auto res = get_no_copy(2);
248285
ASSERT_TRUE(res.ok());

0 commit comments

Comments
 (0)