[Lldb-commits] [PATCH] D151765: [lldb] Introduce the FileSpecBuilder abstraction

[Alex Langford via Phabricator via lldb-commits]

bulbazord planned changes to this revision.
bulbazord added a comment.

In D151765#4385711 <https://reviews.llvm.org/D151765#4385711>, @jingham wrote:

> Why did you choose to have a separate FileSpecBuilder class, rather than making FileSpec smarter about the structure of the path (e.g. have an array of StringRef's into the paths for each component.)   We could do the parse once the first time a path element was requested, and then operations like "RemovePathComponent" would be trivial.  We could maybe even get rid of the distinction between "path" and "filename" since "filename" is really just "last path component".

You mean have FileSpec replace its `ConstString m_directory` and `ConstString m_filename` fields with `llvm::SmallString m_path`? That would indeed avoid something like `FileSpecBuilder` (and probably be simpler in the end). As long as the API of FileSpec stays the same, the changes to call sites should be minimal. The thing I'm mostly concerned about is lifetimes... How often do we grab a ConstString from a FileSpec and not think about the lifetime? Hopefully not many places.

I'll try this out and see what the fallout is. If it's the better way to go, then I'll just refactor all the places where those assumptions are problematic.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151765/new/

https://reviews.llvm.org/D151765