Skip to content

Updated compiler 41 test, handle the exception thrown correctly#2510

Merged
jwillemsen merged 2 commits intomasterfrom
jwi-excepttest
Mar 10, 2026
Merged

Updated compiler 41 test, handle the exception thrown correctly#2510
jwillemsen merged 2 commits intomasterfrom
jwi-excepttest

Conversation

@jwillemsen
Copy link
Member

@jwillemsen jwillemsen commented Mar 10, 2026

Summary by CodeRabbit

  • Tests
    • Enhanced structured exception handling validation: added tracking and logging for an outer exception path, clarified inner finally behavior, and added post-execution checks that report success or failure for both EXCEPT and FINALLY execution.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 10, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3c24c24f-fa0b-4d82-9331-f89f459102b8

📥 Commits

Reviewing files that changed from the base of the PR and between d795777 and 3ea4864.

📒 Files selected for processing (1)
  • ACE/tests/Compiler_Features_41_Test.cpp

Walkthrough

Introduces an outer SEH __except block and an except_executed flag to the existing SEH test, adjusts logging/messages for outer and inner SEH regions, and adds post-run validation checks to confirm both EXCEPT and FINALLY paths execute.

Changes

Cohort / File(s) Summary
SEH Test Enhancement
ACE/tests/Compiler_Features_41_Test.cpp
Adds except_executed boolean and initialization; replaces single __try/__finally flow with __try/__except/__finally; adds distinct log messages for outer SEH_TRY, inner SEH_TRY/FINALLY, and EXCEPT; validates and logs success/failure for FINALLY and EXCEPT execution; minor whitespace/test invocation tweak.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Poem

🐰 I hopped through try, then except I found,
Inner finally hugged the ground,
Flags set true, logs all in sight,
SEH danced well into the night. ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title directly describes the main change: updating a compiler test to properly handle exceptions in the SEH (Structured Exception Handling) flow by adding an except block with exception tracking.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch jwi-excepttest

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ACE/tests/Compiler_Features_41_Test.cpp`:
- Around line 52-56: The outer SEH handler currently uses the broad
EXCEPTION_EXECUTE_HANDLER which will swallow any SEH; change it to filter for
the intended access violation only by using a filtered handler that checks the
exception code (e.g., replace ACE_SEH_EXCEPT (EXCEPTION_EXECUTE_HANDLER) with a
filter that uses GetExceptionCode() == EXCEPTION_ACCESS_VIOLATION or the ACE SEH
filter macro used in Compiler_Features_39_Test.cpp) so only
EXCEPTION_ACCESS_VIOLATION sets except_executed and other faults still
propagate.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6da8cc98-50ee-4f4a-9e96-c0e1732e4bb2

📥 Commits

Reviewing files that changed from the base of the PR and between 71061fa and d795777.

📒 Files selected for processing (1)
  • ACE/tests/Compiler_Features_41_Test.cpp

@jwillemsen
Copy link
Member Author

@Mergifyio backport ace6tao2

@mergify
Copy link

mergify bot commented Mar 10, 2026

backport ace6tao2

✅ Backports have been created

Details

@jwillemsen jwillemsen merged commit de19e72 into master Mar 10, 2026
69 of 71 checks passed
@jwillemsen jwillemsen deleted the jwi-excepttest branch March 10, 2026 15:40
jwillemsen added a commit that referenced this pull request Mar 10, 2026
Updated compiler 41 test, handle the exception thrown correctly (backport #2510)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant