JDevlieghere requested changes to this revision.
JDevlieghere added inline comments.
This revision now requires changes to proceed.
================
Comment at: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm:364
+ Environment env = Host::GetEnvironment();
+ std::string developer_dir = env.lookup("DEVELOPER_DIR");
+ if (developer_dir.empty()) {
----------------
Above we use `getenv`, here we use `Host::GetEnvironment`. We should pick one
and be consistent.
================
Comment at: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm:367
+ // Avoid infinite recursion GetXcodeContentsDirectory calling
GetXcodeSDK.
+ FileSpec path = ::GetXcodeContentsDirectory(false);
+ if (path.RemoveLastPathComponent())
----------------
It looks like this is basically dealing with one case (shlib) of the logic in
GetXcodeContentsDirectory. I would prefer to make this a separate function
`GetXcodeDeveloperDirectory` or something that does just that.
Before I refactored these functions there were several methods where we tried
to share code like this. While I'm generally a strong opponent of sharing code,
this lead to the pattern we see here where we need to add complexity to the
"generic" function to make things work. This in turn makes things more complex
and harder to understand, up till the point that we inevitable implemented
something "similar but different" elsewhere.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81210/new/
https://reviews.llvm.org/D81210
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
