-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Fix the swift-inspect build on Darwin. #41619
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
We had a few typos and such from the recent reorganization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just the one change with GetSymbolAddr
, but the rest seems reasonable. Sorry about the trouble!
|
||
func getAddr(symbolName: String) -> swift_addr_t { | ||
// FIXME: use `__USER_LABEL_PREFIX__` instead of the hardcoded `_`. | ||
let fullName = "_" + symbolName |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason to not use string interpolation?
func getAddr(symbolName: String) -> swift_addr_t { | ||
// FIXME: use `__USER_LABEL_PREFIX__` instead of the hardcoded `_`. | ||
let fullName = "_" + symbolName | ||
var symbol = CSSymbolOwnerGetSymbolWithMangledName(swiftCore, fullName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to use var
rather than let
?
@@ -76,8 +77,8 @@ fileprivate class ConcurrencyDumper { | |||
self.process = process | |||
|
|||
func getMetadata(symbolName: String) -> swift_reflection_ptr_t? { | |||
let addr = process.GetSymbolAddress(symbolName) | |||
if let ptr = process.ReadBytes(addr, MemoryLayout<UInt>.size) { | |||
let addr = process.getAddr(symbolName: symbolName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that this change is less desirable - the GetSymbolAddress
is the generic entry that works across all targets.
I pushed this branch to the main repo by accident, closing in favor of #41620 from my fork. |
We had a few typos and such from the recent reorganization.