[stdlib] Clean up debug printing code #18126
Conversation
|
@swift-ci please test |
airspeedswift
force-pushed
the
debug-cleanup
branch
from
bd8d32b to
d48a907
Compare
| printedElements += 1 | ||
| } | ||
| } | ||
|
|
||
| // LLDB uses this function in expressions, and if it is inlined the resulting | ||
| // LLVM IR is enormous. As a result, to improve LLDB performance we are not | ||
| // making it @inlinable. | ||
| public static func stringForPrintObject(_ value: Any) -> String { | ||
| var maxItemCounter = Int.max |
There was a problem hiding this comment.
This is... quite a big maximum.
| public | ||
| func _debuggerTestingCheckExpect(_ checked_value: String, | ||
| _ expected_value: String) {} | ||
| public func _debuggerTestingCheckExpect(_: String, _: String) { } |
There was a problem hiding this comment.
Seems like this global symbol could be eliminated and users could call into the _DebuggerSupport version directly.
There was a problem hiding this comment.
I think the retain count getters below could be moved into _DebuggerSupport, too, so that stringForPrintObject isn't the only definition there.
There was a problem hiding this comment.
Yes, I think there's no reason to keep this global.
|
@swift-ci please test |
|
@swift-ci please clean test macOS platform |
| public | ||
| func _debuggerTestingCheckExpect(_ checked_value: String, | ||
| _ expected_value: String) {} | ||
| public func _debuggerTestingCheckExpect(_: String, _: String) { } |
There was a problem hiding this comment.
I think the retain count getters below could be moved into _DebuggerSupport, too, so that stringForPrintObject isn't the only definition there.
| public | ||
| func _debuggerTestingCheckExpect(_ checked_value: String, | ||
| _ expected_value: String) {} | ||
| public func _debuggerTestingCheckExpect(_: String, _: String) { } |
There was a problem hiding this comment.
Yes, I think there's no reason to keep this global.
| printedElements += 1 | ||
| } | ||
| } | ||
|
|
||
| // LLDB uses this function in expressions, and if it is inlined the resulting |
There was a problem hiding this comment.
Yep! I'm going to land this, and then will take care of that in a follow-up PR, at the same time as moving those global symbols.
|
Cool. Thanks for the spring cleaning. |
This code has been largely untouched since the early days and has missed out on a few possible improvements since, especially with optionals. Hopefully a NFC, though it doesn't look like it's got full test coverage.
Also removed all inlineable annotations – it oughtn't be necessary to inline any code if it's for debugging, and there's no public generic code to specialize.