root
root copied to clipboard
[cling] Delete UITabCompletion* with unique_ptr to prevent memleak
Changes or fixes:
A minor memleak in cling::UserInterface::runInteractively() reported by valgrind.
Checklist:
- [X] tested changes locally
- [ ] updated the docs (if necessary)
Can one of the admins verify this patch?
@phsft-bot build
Starting build on ROOT-debian10-i386/soversion, ROOT-performance-centos8-multicore/cxx17, ROOT-ubuntu18.04/nortcxxmod, ROOT-ubuntu2004/python3, mac1015/cxx17, mac11/cxx14, windows10/cxx14
How to customize builds
Removed the false comment and moved the unique_ptr declaration above TextInputHolder, in case TextInputHolder::~TextInputHolder() still access it (in the future).
However, I am thinking about enhancing CLI completion support of cling. Such as tab completion. Currently cling only prints completion suggestion but does not complete the input buffer for the user when only single unambiguous completion exists. Also the completion does not recognize meta command like .x, .L so it does not try to complete its arguments (paths). And the path does not expand ~ to home dir.
@Axel-Naumann , do you think it is a good idea for cling to adopt llvm::LineEditor ? (https://llvm.org/doxygen/classllvm_1_1LineEditor.html)
Or building cling's own readline/libedit wrapper?
core/textinput/src/textinput/doc/textinput.txt states that textinput is created to avoid depending on readline/libedit in year 2011.
Is it still true today 2022? cling highly depends on llvm now so it should be fine to adopt llvm::LineEditor (created in 2013. textinput predates it).
I have a prototype cling with the above enhanced completion support based on libedit but I will only create PR if cling (and all root components currently using textinput ) are open to such changes and dependency. I can refactor to use llvm::LineEditor instead. It is not too hard since the latter is also a libedit wrapper.
@Axel-Naumann , @guitargeek Please help approve the github workflow.
Thanks for the PR, @jiangyilism - apologies for being so slow!
I'd be very curious to see llvm's libedit work with ROOT and cling. We can certainly use it for cling - that PR would be very welcome!