Michael137 wrote: > I definitely like this approach much better. Two suggestions: > > * Should this use a FileSpec instead of a `std::string` for the sysroot?
Yea I don't mind doing that. There's no concrete need for it but it's easier to reason about than a raw string I suppose > * I like the simplicity of a `std::pair` but on the other hand it's hard to > tell what the string represents. I think it would help to either store the > `sysroot` in the `XcodeSDK` (potentially as an `std::optional`) or having > this return a `struct` with named fiels (e.g. `xcode_sdk`, `sysroot`). > Putting the sysroot in the XcodeSDK means you don't have to update the > Doxygen comments which are now all outdated. Yea I was toying around with storing it inside `XcodeSDK`. Though it's a bit awkward to have a concrete sysroot path since the `XcodeSDK` class currently represents a merged view of all SDKs (which may have different sysroots). Or at least that's how I interpreted this class. Happy to change it to a named structure. Don't have a strong opinion on this https://github.com/llvm/llvm-project/pull/128712 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
