[Lldb-commits] [PATCH] D156020: [lldb][PlatformDarwin] Parse SDK path for module compilation from debug-info

Michael Buch via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue Jul 25 07:19:10 PDT 2023


Michael137 marked 2 inline comments as not done.
Michael137 added inline comments.


================
Comment at: lldb/include/lldb/Target/Platform.h:479
+    /// to an internal SDK
+    bool found_internal_sdk = false;
+
----------------
Michael137 wrote:
> aprantl wrote:
> > These flags really only make sense in the context of an XcodeSDK, so why not just return an XcodeSDK or XcodeSDK::Info object here? Otherwise we'll probably introduce subtle bugs due to a lossy translation between the flags.
> Yup I think that'd be better. That'll also make it easier to use from the Swift plugin
Actually on second look, the `XcodeSDK` and `XcodeSDK::Info` objects represent information about a single (possibly parsed) SDK path. Whereas what the intention here was is to let the caller know whether we encountered a public/internal SDK while scanning all the CUs. Since we only return a single `XcodeSDK` (not all the ones we looked at) in my opinion it isn't quite right to store that information in it.

This is all really only used to [[ https://github.com/apple/llvm-project/blob/6c39bfc9d521dd8af03ca5e9e6ec7d5d4a6e5e6e/lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp#L1700-L1704 | print a Swift health ]]. Maybe we could instead just log this to `LLDBLog::Types`? Then we don't need to worry about returning any of this information. @aprantl 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156020



More information about the lldb-commits mailing list