Skip to content

Commit 0d19ec1

Browse files
authored
[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.
1 parent 9923816 commit 0d19ec1

1 file changed

Lines changed: 6 additions & 1 deletion

File tree

include/tvm/ffi/extra/structural_visit.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,12 @@ class StructuralWalkCallbackVisitorObj : public StructuralVisitorObj {
572572
if constexpr (order == WalkOrder::kPreOrder) {
573573
auto result = dispatch_(value, this->def_region_kind());
574574
TVM_FFI_S_VISIT_MAYBE_EARLY_RETURN_WITH_ERROR_CONTEXT(result, value);
575-
TVM_FFI_UNSAFE_ASSUME(result.type_index() == TypeIndex::kTVMFFIInt);
575+
// Hoist the call out of TVM_FFI_UNSAFE_ASSUME: clang's -Wassume rejects
576+
// arguments that contain a call expression (its potential side effects
577+
// would be discarded), while [[maybe_unused]] keeps -Wunused-variable
578+
// quiet on configs where the assume macro compiles away.
579+
[[maybe_unused]] int32_t type_index = result.type_index();
580+
TVM_FFI_UNSAFE_ASSUME(type_index == TypeIndex::kTVMFFIInt);
576581
if (TVM_FFI_PREDICT_FALSE(details::ExpectedUnsafe::ValueAs<int32_t>(result) ==
577582
WalkResult::kSkipTag)) {
578583
return details::ExpectedUnsafe::MoveToTVMFFIAny(

0 commit comments

Comments
 (0)