summaryrefslogtreecommitdiff
path: root/CHECK_RETURN_IMPLEMENTATION.md
blob: 31c25ec877a0ee1ea7f99acb473c703b6fbb7ffa (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
# CHECK_RETURN Implementation Complete

## Summary

Implemented Option D (Hybrid) for non-fatal error handling with early return.

## What Was Created

### 1. Header File: `src/util/check_return.h`

**Macros provided:**
```cpp
// Simple error check (90% of cases)
CHECK_RETURN_IF(condition, return_value, "Error: %s", msg);

// Complex error check with cleanup (10% of cases)
CHECK_RETURN_BEGIN(condition, return_value)
  // cleanup code
  ERROR_MSG("Error: %s", msg);
CHECK_RETURN_END

// Warning messages (non-fatal)
WARN_IF(condition, "Warning: %s", msg);
```

**Build behavior:**
- **Debug/Normal:** Full error messages with file:line info
- **STRIP_ALL:** Messages stripped, control flow preserved (saves ~1-2KB)

## What Was Applied

### 2. src/util/asset_manager.cc (3 call sites)

**Before:**
```cpp
if (proc_gen_func_ptr == nullptr) {
  fprintf(stderr, "Error: Unknown procedural function at runtime: %s\n",
          source_record.proc_func_name_str);
  if (out_size) *out_size = 0;
  return nullptr;
}
```

**After:**
```cpp
CHECK_RETURN_BEGIN(proc_gen_func_ptr == nullptr, nullptr)
  if (out_size) *out_size = 0;
  ERROR_MSG("Unknown procedural function at runtime: %s",
            source_record.proc_func_name_str);
  return nullptr;
CHECK_RETURN_END
```

**Applied to:**
- Unknown procedural function check
- Memory allocation failure
- Procedural generation failure

### 3. src/test_demo.cc (2 call sites)

**Before:**
```cpp
if (i + 1 >= argc) {
  fprintf(stderr, "Error: --log-peaks requires a filename argument\n\n");
  print_usage(argv[0]);
  return 1;
}
```

**After:**
```cpp
CHECK_RETURN_BEGIN(i + 1 >= argc, 1)
  print_usage(argv[0]);
  ERROR_MSG("--log-peaks requires a filename argument\n");
  return 1;
CHECK_RETURN_END
```

**Applied to:**
- Missing --log-peaks argument
- Unknown command-line option

## Testing

✅ **All 31 tests pass**
```
100% tests passed, 0 tests failed out of 31
Total Test time (real) =   0.64 sec
```

✅ **Error messages work correctly:**
```bash
$ ./test_demo --invalid-option
Error: Unknown option '--invalid-option'
 [test_demo.cc:199]
Usage: ./test_demo [OPTIONS]
...
```

## Comparison: FATAL_XXX vs CHECK_RETURN

| Aspect | FATAL_XXX | CHECK_RETURN |
|--------|-----------|--------------|
| **Outcome** | `abort()` - program terminates | `return value` - caller handles |
| **Use Case** | Programming errors, invariants | Recoverable errors, validation |
| **Example** | Bounds checks, null dereference | Invalid input, missing files |
| **STRIP_ALL** | Keep checks, strip messages | Strip messages, keep checks |
| **FINAL_STRIP** | Strip everything (0 bytes) | Keep checks (preserves logic) |

## Files Modified

**Created:**
- `src/util/check_return.h` (180 lines)

**Modified:**
- `src/util/asset_manager.cc` (+1 include, 3 call sites refactored)
- `src/test_demo.cc` (+1 include, 2 call sites refactored)

## Size Impact

**Estimated savings with STRIP_ALL:**
- 5 call sites × ~100 bytes per message string = ~500 bytes saved
- Control flow preserved (if-return statements kept)
- No functional changes

## Remaining Opportunities

Can be applied to (optional):
- `src/gpu/texture_manager.cc` - 1 call site
- `src/audio/backend/wav_dump_backend.cc` - 1 call site
- Test files - several call sites

**Total potential:** ~10-15 call sites across codebase

## Usage Guidelines

### When to use CHECK_RETURN_IF:
✅ Simple validation with no cleanup
```cpp
CHECK_RETURN_IF(ptr == nullptr, false, "Invalid pointer");
CHECK_RETURN_IF(size > MAX, -1, "Size too large: %d", size);
```

### When to use CHECK_RETURN_BEGIN/END:
✅ Validation that needs cleanup before return
```cpp
CHECK_RETURN_BEGIN(allocation_failed, nullptr)
  delete[] buffer;
  if (out_size) *out_size = 0;
  ERROR_MSG("Allocation failed: %d bytes", size);
  return nullptr;
CHECK_RETURN_END
```

### When to use WARN_IF:
✅ Non-critical issues that don't prevent execution
```cpp
WARN_IF(config_missing, "Config not found, using defaults");
```

### When to use FATAL_XXX instead:
❌ Don't use CHECK_RETURN for:
- Programming errors (use FATAL_ASSERT)
- Array bounds violations (use FATAL_CHECK)
- Impossible code paths (use FATAL_UNREACHABLE)
- Corrupted state (use FATAL_ERROR)

## Next Steps (Optional)

1. Apply to remaining files (texture_manager.cc, wav_dump_backend.cc)
2. Update CONTRIBUTING.md with CHECK_RETURN guidelines
3. Update AI_RULES.md if needed
4. Consider FINAL_STRIP policy (strip checks vs keep checks)

## Documentation

Full documentation in header file:
- Usage examples
- Build mode behavior
- Comparison with FATAL_XXX
- Size impact analysis