-
Notifications
You must be signed in to change notification settings - Fork 102
Nesting non-failable in handled failable functions yields warnings #894
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Nesting non-failable in handled failable functions yields warnings #894
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request fixes issue #856 where nesting non-failable functions inside failable functions with trust modifiers incorrectly generated warnings. The changes refactor context management to use derive macros for cleaner and more maintainable code.
Key changes:
- Refactored trust context management using
ContextManagerandContextHelperderive macros for automatic context restoration - Wrapped function invocation and command typechecking in
use_modifiersto properly propagate trust context to nested calls - Replaced manual
std::mem::swapusage with generated helper methods (with_silenced,set_is_trust_ctx)
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/context.rs | Added #[context] attribute to is_trust_ctx field to enable automatic context management |
| src/modules/command/modifier.rs | Refactored trust context handling using ContextManager derive macro and set_is_trust_ctx setter |
| src/modules/function/invocation.rs | Wrapped typecheck logic in use_modifiers to propagate trust context; refactored silencing to use with_silenced helper |
| src/modules/command/cmd.rs | Wrapped typecheck logic in use_modifiers to properly handle trust context in commands |
| src/modules/builtin/mv.rs | Refactored silencing management to use with_silenced helper method |
| src/tests/warning/unnecessary_handler.ab | Added test case for warning when non-failable function has failure handler |
| src/tests/warning/nested_trust.ab | Added test case for nested non-failable function in trust block (should not warn) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
…ons-yields-warnings' of https://github.com/Ph0enixKM/AmberNative into 856-bug-nesting-non-failable-in-handled-failable-functions-yields-warnings
|
I'll resolve conflicts later today |
Closes #856