fix(translate): validate control flow stack before popping

[tetsuo-cpp]

Summary

• Add empty-stack guards and tag validation before each cfStack.pop_back_val() in the Forth translator
• Mismatched control flow words (e.g. THEN without IF, REPEAT without WHILE) now produce clear error messages instead of crashing

Test plan

• Added 7 negative test cases covering empty-stack and tag-mismatch scenarios
• All 91 tests pass

Closes#21

[tetsuo-cpp]

Code Review

Looks good. Clean, well-scoped defensive fix with solid test coverage. All 6 cfStack.pop_back_val() sites are now guarded, the tag checks are correct against each push site, and the error messages are clear.

Minor suggestions

1. Missing test for REPEAT tag mismatch: All REPEAT tests trigger the empty-stack path. There's no test exercising the destTag != CFTag::Dest or origTag != CFTag::Orig branches inside REPEAT (e.g. IF REPEAT would hit the first). Not a blocker — the logic is identical to the other sites which are tested for mismatch — but would give full branch coverage.

2. Future follow-up: No end-of-body check for unclosed control flow (e.g. IF without THEN leaves the stack non-empty at end of parsing). Out of scope here but could be a follow-up issue.

[tetsuo-cpp]

Summary

• Add empty-stack guards and tag validation before each cfStack.pop_back_val() in the Forth translator
• Mismatched control flow words (e.g. THEN without IF, REPEAT without WHILE) now produce clear error messages instead of crashing

Test plan

• Added 7 negative test cases covering empty-stack and tag-mismatch scenarios
• All 91 tests pass

Closes#21

[tetsuo-cpp]

Code Review

Looks good. Clean, well-scoped defensive fix with solid test coverage. All 6 cfStack.pop_back_val() sites are now guarded, the tag checks are correct against each push site, and the error messages are clear.

Minor suggestions

1. Missing test for REPEAT tag mismatch: All REPEAT tests trigger the empty-stack path. There's no test exercising the destTag != CFTag::Dest or origTag != CFTag::Orig branches inside REPEAT (e.g. IF REPEAT would hit the first). Not a blocker — the logic is identical to the other sites which are tested for mismatch — but would give full branch coverage.

2. Future follow-up: No end-of-body check for unclosed control flow (e.g. IF without THEN leaves the stack non-empty at end of parsing). Out of scope here but could be a follow-up issue.

[tetsuo-cpp]

Summary

• Add empty-stack guards and tag validation before each cfStack.pop_back_val() in the Forth translator
• Mismatched control flow words (e.g. THEN without IF, REPEAT without WHILE) now produce clear error messages instead of crashing

Test plan

• Added 7 negative test cases covering empty-stack and tag-mismatch scenarios
• All 91 tests pass

Closes#21

[tetsuo-cpp]

Code Review

Looks good. Clean, well-scoped defensive fix with solid test coverage. All 6 cfStack.pop_back_val() sites are now guarded, the tag checks are correct against each push site, and the error messages are clear.

Minor suggestions

1. Missing test for REPEAT tag mismatch: All REPEAT tests trigger the empty-stack path. There's no test exercising the destTag != CFTag::Dest or origTag != CFTag::Orig branches inside REPEAT (e.g. IF REPEAT would hit the first). Not a blocker — the logic is identical to the other sites which are tested for mismatch — but would give full branch coverage.

2. Future follow-up: No end-of-body check for unclosed control flow (e.g. IF without THEN leaves the stack non-empty at end of parsing). Out of scope here but could be a follow-up issue.

[tetsuo-cpp]

Summary

• Add empty-stack guards and tag validation before each cfStack.pop_back_val() in the Forth translator
• Mismatched control flow words (e.g. THEN without IF, REPEAT without WHILE) now produce clear error messages instead of crashing

Test plan

• Added 7 negative test cases covering empty-stack and tag-mismatch scenarios
• All 91 tests pass

Closes#21

[tetsuo-cpp]

Code Review

Looks good. Clean, well-scoped defensive fix with solid test coverage. All 6 cfStack.pop_back_val() sites are now guarded, the tag checks are correct against each push site, and the error messages are clear.

Minor suggestions

1. Missing test for REPEAT tag mismatch: All REPEAT tests trigger the empty-stack path. There's no test exercising the destTag != CFTag::Dest or origTag != CFTag::Orig branches inside REPEAT (e.g. IF REPEAT would hit the first). Not a blocker — the logic is identical to the other sites which are tested for mismatch — but would give full branch coverage.

2. Future follow-up: No end-of-body check for unclosed control flow (e.g. IF without THEN leaves the stack non-empty at end of parsing). Out of scope here but could be a follow-up issue.

[tetsuo-cpp]

Summary

• Add empty-stack guards and tag validation before each cfStack.pop_back_val() in the Forth translator
• Mismatched control flow words (e.g. THEN without IF, REPEAT without WHILE) now produce clear error messages instead of crashing

Test plan

• Added 7 negative test cases covering empty-stack and tag-mismatch scenarios
• All 91 tests pass

Closes#21

[tetsuo-cpp]

Code Review

Looks good. Clean, well-scoped defensive fix with solid test coverage. All 6 cfStack.pop_back_val() sites are now guarded, the tag checks are correct against each push site, and the error messages are clear.

Minor suggestions

1. Missing test for REPEAT tag mismatch: All REPEAT tests trigger the empty-stack path. There's no test exercising the destTag != CFTag::Dest or origTag != CFTag::Orig branches inside REPEAT (e.g. IF REPEAT would hit the first). Not a blocker — the logic is identical to the other sites which are tested for mismatch — but would give full branch coverage.

2. Future follow-up: No end-of-body check for unclosed control flow (e.g. IF without THEN leaves the stack non-empty at end of parsing). Out of scope here but could be a follow-up issue.

[tetsuo-cpp]

Summary

• Add empty-stack guards and tag validation before each cfStack.pop_back_val() in the Forth translator
• Mismatched control flow words (e.g. THEN without IF, REPEAT without WHILE) now produce clear error messages instead of crashing

Test plan

• Added 7 negative test cases covering empty-stack and tag-mismatch scenarios
• All 91 tests pass

Closes#21

[tetsuo-cpp]

Code Review

Looks good. Clean, well-scoped defensive fix with solid test coverage. All 6 cfStack.pop_back_val() sites are now guarded, the tag checks are correct against each push site, and the error messages are clear.

Minor suggestions

1. Missing test for REPEAT tag mismatch: All REPEAT tests trigger the empty-stack path. There's no test exercising the destTag != CFTag::Dest or origTag != CFTag::Orig branches inside REPEAT (e.g. IF REPEAT would hit the first). Not a blocker — the logic is identical to the other sites which are tested for mismatch — but would give full branch coverage.

2. Future follow-up: No end-of-body check for unclosed control flow (e.g. IF without THEN leaves the stack non-empty at end of parsing). Out of scope here but could be a follow-up issue.

[tetsuo-cpp]

Summary

• Add empty-stack guards and tag validation before each cfStack.pop_back_val() in the Forth translator
• Mismatched control flow words (e.g. THEN without IF, REPEAT without WHILE) now produce clear error messages instead of crashing

Test plan

• Added 7 negative test cases covering empty-stack and tag-mismatch scenarios
• All 91 tests pass

Closes#21

[tetsuo-cpp]

Code Review

Looks good. Clean, well-scoped defensive fix with solid test coverage. All 6 cfStack.pop_back_val() sites are now guarded, the tag checks are correct against each push site, and the error messages are clear.

Minor suggestions

1. Missing test for REPEAT tag mismatch: All REPEAT tests trigger the empty-stack path. There's no test exercising the destTag != CFTag::Dest or origTag != CFTag::Orig branches inside REPEAT (e.g. IF REPEAT would hit the first). Not a blocker — the logic is identical to the other sites which are tested for mismatch — but would give full branch coverage.

2. Future follow-up: No end-of-body check for unclosed control flow (e.g. IF without THEN leaves the stack non-empty at end of parsing). Out of scope here but could be a follow-up issue.

[tetsuo-cpp]

Summary

• Add empty-stack guards and tag validation before each cfStack.pop_back_val() in the Forth translator
• Mismatched control flow words (e.g. THEN without IF, REPEAT without WHILE) now produce clear error messages instead of crashing

Test plan

• Added 7 negative test cases covering empty-stack and tag-mismatch scenarios
• All 91 tests pass

Closes#21

[tetsuo-cpp]

Code Review

Looks good. Clean, well-scoped defensive fix with solid test coverage. All 6 cfStack.pop_back_val() sites are now guarded, the tag checks are correct against each push site, and the error messages are clear.

Minor suggestions

1. Missing test for REPEAT tag mismatch: All REPEAT tests trigger the empty-stack path. There's no test exercising the destTag != CFTag::Dest or origTag != CFTag::Orig branches inside REPEAT (e.g. IF REPEAT would hit the first). Not a blocker — the logic is identical to the other sites which are tested for mismatch — but would give full branch coverage.

2. Future follow-up: No end-of-body check for unclosed control flow (e.g. IF without THEN leaves the stack non-empty at end of parsing). Out of scope here but could be a follow-up issue.