feat: onCleanup functionality#1503
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements the onCleanup functionality, which allows automatic execution of cleanup tasks when a function completes (either normally or through an exception). The implementation follows a handle-based design pattern similar to other handle types in the Nelson codebase.
Key Changes:
- Adds
OnCleanupObjectHandleclass to manage cleanup tasks with function handles - Integrates cleanup execution into
MacroFunctionDeflifecycle (both normal and exception paths) - Provides
cancel()method to prevent cleanup execution - Includes comprehensive test coverage and bilingual documentation
Reviewed changes
Copilot reviewed 43 out of 43 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/types/src/include/Types.hpp | Adds handle category constant for onCleanup objects |
| modules/interpreter/src/include/OnCleanupObjectHandle.hpp | Defines the OnCleanupObjectHandle class with cleanup and cancel methods |
| modules/interpreter/src/cpp/OnCleanupObjectHandle.cpp | Implements cleanup execution and cancellation logic |
| modules/interpreter/src/include/MacroFunctionDef.hpp | Adds cleanup task tracking to function definitions |
| modules/interpreter/src/cpp/MacroFunctionDef.cpp | Integrates onCleanup execution into function lifecycle (normal and exception paths) |
| modules/interpreter/src/include/DeleteOnCleanupHandleObject.hpp | Declares deletion helper for cleanup handles |
| modules/interpreter/src/cpp/DeleteOnCleanupHandleObject.cpp | Implements cleanup handle deletion with automatic cleanup execution |
| modules/interpreter/builtin/cpp/onCleanupBuiltin.cpp | Main constructor that creates and registers cleanup objects |
| modules/interpreter/builtin/cpp/onCleanup_cancelBuiltin.cpp | Implements cancel method builtin |
| modules/interpreter/builtin/cpp/onCleanup_deleteBuiltin.cpp | Implements delete method builtin |
| modules/interpreter/builtin/cpp/onCleanup_getBuiltin.cpp | Implements property getter builtin |
| modules/interpreter/builtin/cpp/onCleanup_usedBuiltin.cpp | Tracks handle usage statistics |
| modules/interpreter/builtin/cpp/Gateway.cpp | Registers all onCleanup-related builtins |
| modules/interpreter/functions/@onCleanup/display.m | Implements display function for onCleanup objects |
| modules/interpreter/functions/@onCleanup/disp.m | Implements disp function showing task property |
| modules/interpreter/etc/startup.m | Adds functions directory to path |
| modules/interpreter/etc/finish.m | Removes functions directory from path |
| modules/interpreter/module.iss | Enables recursive installation of functions directory |
| modules/interpreter/tests/test_onCleanup.m | Main test suite covering normal, explicit, and cancelled cleanup |
| modules/interpreter/tests/onCleanup/demo_onCleanup.m | Demo function used by tests |
| modules/interpreter/help/en_US/xml/onCleanup.xml | English documentation |
| modules/interpreter/help/fr_FR/xml/onCleanup.xml | French documentation |
| modules/handle/builtin/cpp/cancelBuiltin.cpp | Generic cancel builtin requiring overload |
| modules/handle/builtin/cpp/Gateway.cpp | Registers cancel as a generic handle method |
| modules/parallel/builtin/cpp/Gateway.cpp | Moves cancel from generic to overloaded methods for Future types |
| modules/history_manager/builtin/cpp/historyBuiltin.cpp | Adds interrupt checking to loops (unrelated enhancement) |
| modules/types/src/c/nlsTypes.vcxproj* | Removes duplicate nlsConfig.h reference |
| modules/interpreter/CMakeLists.txt | Adds handle module dependency and removes duplicate include |
| modules/interpreter/src/c/nlsInterpreter.vcxproj* | Adds new source files to build |
| modules/interpreter/builtin/c/nlsInterpreter_builtin.vcxproj* | Adds new builtin files and dependencies |
| modules/handle/builtin/c/nlsHandle_builtin.vcxproj* | Adds cancel builtin to build |
| modules/functions_manager/src/include/ClearFunction.hpp | Adds formatting separators (cosmetic) |
| CHANGELOG.md | Documents new onCleanup feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| void | ||
| MacroFunctionDef::onCleanup(Evaluator* eval) | ||
| { | ||
| Context* context = eval->getContext(); |
Check notice
Code scanning / CodeQL
Unused local variable Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
To fix the unused local variable issue, simply remove the declaration and initialization of context from line 126 in the MacroFunctionDef::onCleanup method. Do not remove or modify any other lines, since context is not used elsewhere and its removal will not affect existing functionality. No changes to imports or other definitions are necessary.
| @@ -123,7 +123,6 @@ | ||
| void | ||
| MacroFunctionDef::onCleanup(Evaluator* eval) | ||
| { | ||
| Context* context = eval->getContext(); | ||
| for (auto task : cleanupTasks) { | ||
| if (task.isHandle() && task.getHandleClassName() == NLS_HANDLE_ONCLEANUP_CATEGORY_STR) { | ||
| OnCleanupObjectHandle* obj = (OnCleanupObjectHandle*)task.getContentAsHandleScalar(); |
| return; | ||
| } | ||
| function_handle fh = task.getContentAsFunctionHandle(); | ||
| AnonymousMacroFunctionDef* cp |
Check notice
Code scanning / CodeQL
Unused local variable Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
To fix the unused local variable error, simply remove the declaration and assignment of the variable cp on line 32. This change should be made in modules/interpreter/src/cpp/OnCleanupObjectHandle.cpp, inside the body of the OnCleanupObjectHandle::cleanup function. No other adjustments or imports are necessary, as cp is not used elsewhere and its removal does not affect the logic of the function.
| @@ -29,8 +29,6 @@ | ||
| return; | ||
| } | ||
| function_handle fh = task.getContentAsFunctionHandle(); | ||
| AnonymousMacroFunctionDef* cp | ||
| = reinterpret_cast<AnonymousMacroFunctionDef*>(fh.anonymousHandle); | ||
| FunctionDef* funcDef = nullptr; | ||
| if (fh.anonymousHandle != nullptr) { | ||
| funcDef = (FunctionDef*)fh.anonymousHandle; |
3921123 to
a9d38b5
Compare
a9d38b5 to
b70a12b
Compare
No description provided.