summaryrefslogtreecommitdiff
path: root/CHECK_RETURN_IMPLEMENTATION.md
diff options
context:
space:
mode:
authorskal <pascal.massimino@gmail.com>2026-02-08 16:04:53 +0100
committerskal <pascal.massimino@gmail.com>2026-02-08 16:04:53 +0100
commit06c8c665f0a9e5ac3819004e8d64b1fa323deeb0 (patch)
treefec7d278b1afe7292556415c0fd7244a0d89113b /CHECK_RETURN_IMPLEMENTATION.md
parent0f99e09bd4a4bd73c25d0bba9e1954c911fd8be8 (diff)
feat(util): Add CHECK_RETURN macros for recoverable errors
Created check_return.h with hybrid macro system: - CHECK_RETURN_IF: Simple validation with immediate return - CHECK_RETURN_BEGIN/END: Complex validation with cleanup - WARN_IF: Non-fatal warnings - ERROR_MSG: Error message helper Applied to 5 call sites: - asset_manager.cc: 3 procedural generation errors - test_demo.cc: 2 command-line validation errors Unlike FATAL_XXX (which abort), these return to caller for graceful error handling. Messages stripped in STRIP_ALL, control flow preserved. Size impact: ~500 bytes saved in STRIP_ALL builds Tests: 31/31 passing Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Diffstat (limited to 'CHECK_RETURN_IMPLEMENTATION.md')
-rw-r--r--CHECK_RETURN_IMPLEMENTATION.md181
1 files changed, 181 insertions, 0 deletions
diff --git a/CHECK_RETURN_IMPLEMENTATION.md b/CHECK_RETURN_IMPLEMENTATION.md
new file mode 100644
index 0000000..31c25ec
--- /dev/null
+++ b/CHECK_RETURN_IMPLEMENTATION.md
@@ -0,0 +1,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