[Lldb-commits] [PATCH] D151399: [lldb] Introduce FileSpec::GetComponents

Thu May 25 10:17:21 PDT 2023

bulbazord added inline comments.

================
Comment at: lldb/include/lldb/Utility/FileSpec.h:420
+  ///   A std::vector of std::strings for each path component.
+  std::vector<std::string> GetComponents() const;
+
----------------
JDevlieghere wrote:
> bulbazord wrote:
> > JDevlieghere wrote:
> > > I'm surprised this returns a vector of `std::string`s and not `llvm::StringRef`s. I would expect all the components to be part of the FileSpec's stored path. Even with the file and directory stored as separate `ConstString`s, that should be possible?
> > Yes, it is possible to do `std::vector<llvm::StringRef>` here, especially because they would be backed by `ConstString`s which live forever. I chose `std::string` here because of the possibility that we one day no longer use `ConstString` to store the path, in which case the vector's StringRefs would only be valid for the lifetime of the FileSpec (or until it gets mutated).
> > 
> > Maybe I'm thinking too far ahead or planning for a future that will never come though. What do you think?
> I think it's reasonable to expect the lifetime of things handed out by a FileSpec match the lifetime of the FileSpec, but that depends on how we want to deal with mutability. If we want to be able to mutate a FileSpec in place, then you're right, these things need to have their own lifetime. 
I'd prefer to move towards FileSpec being immutable (or as close as possible), so I'll update this and change it to `StringRef. Thanks for the feedback!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151399