[Lldb-commits] [lldb] [lldb][HostInfoMacOSX] Search CommandLineTools directory when looking up SDK paths (PR #128712)

[Michael Buch via lldb-commits]

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