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

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

Reply via email to