diff options
Diffstat (limited to 'CHECK_RETURN_IMPLEMENTATION.md')
| -rw-r--r-- | CHECK_RETURN_IMPLEMENTATION.md | 181 |
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 |
