[cling] Replace usage of TextInput with LineEditor
This Pull request:
- Replace all occurrences of TextInput with LLVM's LineEditor library in
CLING. This moves the codebase closer to clang-repl and might help in future changes. This will help in removing this whole folder fromCLING- https://github.com/root-project/cling/tree/master/lib/UserInterface/textinput, but this doesn't exist in theROOTrepository. - Corresponding replacement should also be made in
ROOTfor complete removal of TextInput, working on it. Will not be as straightforward as this. - LineEditor does not support setting history size manually (with
CLING_HISTSIZE). For now added this functionality in LLVM; Can later upstream or deprecate this functionality (clang-repl does not have this feature, AFAIK). But ROOT might be relying on this feature - need to look into it. Diff LLVM will fail due to this. - A couple of tests were adjusted to make the tests pass.
- I tried to keep the changes minimal, so we get a proper diff. It can be made more similar to
clang-repllater.
Changes or fixes:
Checklist:
- [x] tested changes locally
- [ ] updated the docs (if necessary)
This PR fixes #
Test Results
22 files 22 suites 4d 0h 43m 44s ⏱️ 3 707 tests 3 706 ✅ 0 💤 1 ❌ 79 603 runs 79 602 ✅ 0 💤 1 ❌
For more details on these failures, see this check.
Results for commit 1c3c7297.
:recycle: This comment has been updated with latest results.
We need to decide what to do with
CLING_HISTSIZE. If we want to support it, the necessary changes should be upstreamed to LLVM first and then backported to our fork. Note that it would be a first and prevent building against a vanilla LLVM!
This was implemented to be able to set the history to 0 so that we do not get different pointers across gdb runs, iirc.
We need to decide what to do with
CLING_HISTSIZE. If we want to support it, the necessary changes should be upstreamed to LLVM first and then backported to our fork. Note that it would be a first and prevent building against a vanilla LLVM!
From yesterday's discussion, it seems that this does not align with the idea of using a prebuilt vanilla LLVM. I'm tending towards deprecating CLING_HISTSIZE. The functionality was added here: https://github.com/root-project/root/pull/11286
Regardless, I think the ability to set a history size would be a nice feature for all the tools using it. So, I will try to upstream this anyways. LineEditor hardcodes history size to 800: https://github.com/root-project/root/blob/99a126187f79163b7f5bfc506fa15903f784efbc/interpreter/llvm-project/llvm/lib/LineEditor/LineEditor.cpp#L224
We need to decide what to do with
CLING_HISTSIZE. If we want to support it, the necessary changes should be upstreamed to LLVM first and then backported to our fork. Note that it would be a first and prevent building against a vanilla LLVM!This was implemented to be able to set the history to 0 so that we do not get different pointers across gdb runs, iirc.
Disabling the history can probably be implemented differently, maybe that's enough.
We need to decide what to do with
CLING_HISTSIZE. If we want to support it, the necessary changes should be upstreamed to LLVM first and then backported to our fork. Note that it would be a first and prevent building against a vanilla LLVM!This was implemented to be able to set the history to 0 so that we do not get different pointers across gdb runs, iirc.
Disabling the history can probably be implemented differently, maybe that's enough.
LineEditor doesn't seem to provide a way to do it though. I've opened a PR here: https://github.com/llvm/llvm-project/pull/110092
This is marked draft because standalone cling repository will need textinput directory removed (which can go later but will need a separate commit in the standalone repository)