Skip to content

Phase 4: -Wunused-variable を潰して -Werror=unused-variable を有効化#58

Merged
thawk105 merged 1 commit into
masterfrom
werror-unused-variable
May 13, 2026
Merged

Phase 4: -Wunused-variable を潰して -Werror=unused-variable を有効化#58
thawk105 merged 1 commit into
masterfrom
werror-unused-variable

Conversation

@thawk105
Copy link
Copy Markdown
Owner

#43 Phase 4 のサブタスク。-Wunused-variable 167 件 (file:line:col ユニーク、36 ソースファイル) を潰し、ccbench_add_protocol()-Werror=unused-variable を追加する。

真のバグ 2 件

1. tx.read() の戻り値 check 漏れ → stale body deref

include/bomb.hhinclude/bomb_pessimistic.hhget_material_cost() で、tx.read()Statusstat で受けていたが unused のままで、tx.status_ == aborted だけ確認して body->get_value().cast_to<MaterialCostMaster>() を呼んでいた。CLAUDE.md の "tx.read returns Status — check it" 節そのままのパターンで、MaterialCostMaster 行が無いと前回 read の stale body を deref する。両方に if (stat != Status::OK) return false; を追加。

2. cc/oze/{bomb,ycsb}_oze.ccactual_extime print が漏れていた

他の protocol の workload binary (silo / mocc / mvto / si / ss2pl / ermia / cicada / tictoc) は全て ShowOptParameters() の後に std::cout << \"actual_extime:\\t\" << actual_extime << std::endl; を出力しているが、oze の bomb / ycsb 版だけこの行が無く、actual_extime が計算するだけで捨てられていた (tpcc_oze.cc は正しく出力している)。print を追加して他の binary と出力スキーマを揃えた。

カテゴリ別の修正方針

カテゴリ 件数 (おおむね) 方針
真のバグ (tx.read の Status 無視 + stale body deref) 2 if (stat != Status::OK) return false; を追加
観測性の漏れ (actual_extime の print) 2 print 行を追加 (他 binary と統一)
Result &myres = std::ref(...)<workload>_<proto>.cc の worker で完全 dead 14 削除
uint64_t epoch_timer_start, epoch_timer_stop; が oze の workload entry で dead 6 削除
Status stat;run() 先頭で declare されて未使用 (bomb.hh, bomb_pessimistic.hh) 2 削除
Version *expected, *desired;expected が ermia / si の delete_record で dead 2 expected を分離
SetElement<Tuple>* re; 等の dead 宣言 (ermia / si / oze) 多数 削除
bool isInvisible; (oze) 3 削除
int num_pages = 0; int num_skip_propagate = 0; (oze) 2 削除
ループ内で shadowing する dead 宣言 (uint32_t p_id, SimpleKey<8> key) 3+ 削除
auto n = tx.read_set_.size(); 等の dead snapshot 2 削除
副作用が目的の値を materialize する &t = ... (YCSB の read/write workload、OrderLine の touch) 数件 [[maybe_unused]]
単に N 回繰り返すだけのループの induction var (for (auto& k : keys)) 2 [[maybe_unused]]
意図的に return を捨てている関数呼び出し (tx.scan, tx.delete_record, Masstrees[...].remove_value, read_internal, select_im_by_factory, select_pc_by_factory) 多数 (void) + 短いコメント (Phase 1 #47select_im_by_factory のパターンを踏襲)

remove_value(void) 化は silo / mocc / tictoc / ss2pl / d2pl の writePhase DELETE branch で同じ shape のコードを 5 個別個に直しているが、共通化はこの PR のスコープ外とする。

CMake

 target_compile_options(${target} PRIVATE
   -Werror=maybe-uninitialized
   -Werror=unused-but-set-variable
-  -Werror=unused-label)
+  -Werror=unused-label
+  -Werror=unused-variable)

Test plan

  • GCC 13 Release / -DENABLE_SANITIZER=OFF: 34/34 binaries built, 0 warnings
  • GCC 11 Debug + ASan: 34/34 binaries built
  • CI 緑

Related

@thawk105 thawk105 force-pushed the werror-unused-variable branch from 90f9475 to 1821add Compare May 13, 2026 14:33
Promotes -Wunused-variable to error in ccbench_add_protocol() and
clears the 167 GCC 13 hits (unique file:line:col, across 36 source
files) it surfaces. Continues the per-flag rollout for #43.

Two of the hits were real latent bugs and got proper fixes; the rest
are dead-receive locals (remove) or genuinely-discarded return values
(make explicit with `(void)`).

1. include/bomb.hh `get_material_cost()` and the identical copy in
   include/bomb_pessimistic.hh both held the return of `tx.read()` in
   an unused `stat`, then only checked `tx.status_ == aborted` before
   dereferencing `body`. Per CLAUDE.md "tx.read returns Status - check
   it", `*body` is left untouched on WARN_NOT_FOUND, so a missing
   MaterialCostMaster row would have caused a stale-pointer
   `cast_to<MaterialCostMaster>` deref. Added the missing
   `if (stat != Status::OK) return false;` line in both copies.

2. cc/oze/{bomb,ycsb}_oze.cc computed `actual_extime` but never printed
   it, while every other workload binary (silo, mocc, mvto, si, ss2pl,
   ermia, cicada, tictoc) prints the line `"actual_extime:\t<value>"`
   after `ShowOptParameters()`. Added the missing print to keep the
   oze output schema consistent with the rest.

- Dead local variables (`Result &myres = std::ref(...)`,
  `uint64_t epoch_timer_start/stop`, `Status stat;` at the top of
  long functions, `Version *expected`, `SetElement<Tuple>* re/we`,
  `bool isInvisible`, `int num_pages / num_skip_propagate`,
  `uint32_t p_id` shadowing in load loops, `SimpleKey<8> key`
  re-declarations inside loops, `auto n = tx.read_set_.size()`
  snapshots): removed.
- Workload-loop temporaries that materialize a value as a deliberate
  side effect (the YCSB read+touch / write+materialize pattern in
  include/ycsb.hh, and the OrderLine touch in
  include/tpcc/tpcc_tx_orderstatus.hh): annotated `[[maybe_unused]]`.
- Loop induction vars whose only role is N-iterations bookkeeping
  (`for (const auto& k : keys)` in cc/oze/transaction.cc where the
  body operates on the whole container, not on `k`): annotated
  `[[maybe_unused]]`. The redundant inner-loop body is preserved as-is
  to keep this PR scoped to the warning.
- Discarded return values from functions whose side effect is what
  the caller wants (`tx.scan`, `tx.delete_record`,
  `Masstrees[...].remove_value`, `read_internal`,
  `select_im_by_factory`, `select_pc_by_factory`): made explicit with
  `(void)` and a brief comment, mirroring the pattern Phase 1 (#47)
  used for `select_im_by_factory` in BoMB.

The `remove_value` `(void)` cast is duplicated across the
silo/mocc/tictoc/ss2pl/d2pl writePhase DELETE branches because they
all share the same code shape; consolidating them is out of scope
here.

```diff
 target_compile_options(${target} PRIVATE
   -Werror=maybe-uninitialized
   -Werror=unused-but-set-variable
-  -Werror=unused-label)
+  -Werror=unused-label
+  -Werror=unused-variable)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant