[Lldb-commits] [PATCH] D49740: Move RegisterValue, Scalar, State from Core to Utility

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D49740#1188079, @teemperor wrote:

> > CompletionRequest - this sounds like it could go next to the command interpreter
>
> Yeah, makes sense. Even though Utility classes then can either not offer completion methods (currently only ArchSpec is doing that) or work around that with a forward decl. But I think that's not a big issue.


I am not too bothered by that either, but I think that for the thing that the ArchSpec function is doing, passing it the entire CompletionRequest is overkill. Ideally, I'd set things up such that ArchSpec and friends don't have any knowledge of "completion" -- they would just offer an interface for accessing all possible values(*), and then completion could be built on top of that. This way the data source can be independent of the actual algorithm computing the completions.

(*) The interface could be as simple as an iterator over all ArchSpec string values. However, this may not be good enough in terms of performance (it wouldn't matter for ArchSpec, but it might for some other classes), in which case we might want to do something slightly more fancy (`GiveMeAllValuesStartingWith("foo")`).


https://reviews.llvm.org/D49740