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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to