This is an automated email from the ASF dual-hosted git repository.
tqchen pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tvm-ffi.git
The following commit(s) were added to refs/heads/main by this push:
new 0d19ec1 [FIX] Avoid -Wassume error in structural visit VisitImpl
(#632)
0d19ec1 is described below
commit 0d19ec173c12dbeda41b5d2541d31d83b6ad1160
Author: Yaxing Cai <[email protected]>
AuthorDate: Thu Jun 18 22:37:55 2026 +0800
[FIX] Avoid -Wassume error in structural visit VisitImpl (#632)
## Problem
The macOS clang CI job on `main` fails to compile with `-Werror`:
```
include/tvm/ffi/extra/structural_visit.h:575:29: error: the argument to
'__builtin_assume' has side effects that will be discarded
[-Werror,-Wassume]
TVM_FFI_UNSAFE_ASSUME(result.type_index() == TypeIndex::kTVMFFIInt);
```
## Root cause
clang's `__builtin_assume` discards its argument's side effects, so
`-Wassume` rejects any argument that **contains a call expression** —
this is a purely syntactic check, independent of whether the call is
actually pure.
This is a regression from #629, which inlined `result.type_index()`
directly into `TVM_FFI_UNSAFE_ASSUME` to silence `-Wunused-variable`.
The two warnings pull in opposite directions:
| | `-Wunused-variable` | `-Wassume` |
|---|---|---|
| pre-#629 (local var) | ❌ (when assume macro is a no-op) | ✅ |
| #629 (inlined call) | ✅ | ❌ ← CI fails |
| this PR | ✅ | ✅ |
Note `result` here is a C++ `Expected`, so `result.type_index()` is a
**method call**. The other 10 `TVM_FFI_UNSAFE_ASSUME` sites in the tree
operate on the raw `TVMFFIAny` C struct via the plain field
`src->type_index`, which contains no call expression and is therefore
unaffected. This was the only call-expression assume site in the
codebase.
## Fix
Hoist the call back into a local and mark it `[[maybe_unused]]`:
```cpp
[[maybe_unused]] int32_t type_index = result.type_index();
TVM_FFI_UNSAFE_ASSUME(type_index == TypeIndex::kTVMFFIInt);
```
- The assume argument is now a plain variable read → satisfies
`-Wassume`.
- `[[maybe_unused]]` keeps `-Wunused-variable` quiet on configs where
the assume macro compiles away to a no-op → satisfies what #629 was
originally fixing.
## Validation
- `clang++ -std=c++17 -Werror -Wassume -fsyntax-only
src/ffi/extra/structural_visit.cc` (Homebrew clang 22): **passes** after
the change, **fails** before it (reproduces the CI error).
- Confirmed `[[maybe_unused]]` suppresses `-Wunused-variable` when the
assume macro expands to a no-op.
- `clang-format` clean; lines within the 100-col limit.
---
include/tvm/ffi/extra/structural_visit.h | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/tvm/ffi/extra/structural_visit.h
b/include/tvm/ffi/extra/structural_visit.h
index f585573..7a147cf 100644
--- a/include/tvm/ffi/extra/structural_visit.h
+++ b/include/tvm/ffi/extra/structural_visit.h
@@ -572,7 +572,12 @@ class StructuralWalkCallbackVisitorObj : public
StructuralVisitorObj {
if constexpr (order == WalkOrder::kPreOrder) {
auto result = dispatch_(value, this->def_region_kind());
TVM_FFI_S_VISIT_MAYBE_EARLY_RETURN_WITH_ERROR_CONTEXT(result, value);
- TVM_FFI_UNSAFE_ASSUME(result.type_index() == TypeIndex::kTVMFFIInt);
+ // Hoist the call out of TVM_FFI_UNSAFE_ASSUME: clang's -Wassume rejects
+ // arguments that contain a call expression (its potential side effects
+ // would be discarded), while [[maybe_unused]] keeps -Wunused-variable
+ // quiet on configs where the assume macro compiles away.
+ [[maybe_unused]] int32_t type_index = result.type_index();
+ TVM_FFI_UNSAFE_ASSUME(type_index == TypeIndex::kTVMFFIInt);
if
(TVM_FFI_PREDICT_FALSE(details::ExpectedUnsafe::ValueAs<int32_t>(result) ==
WalkResult::kSkipTag)) {
return details::ExpectedUnsafe::MoveToTVMFFIAny(
