Log not lldbassert when the NSNumber summary formatter encounters a "preserved" number #2798
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The ObjC runtime offers both signed & unsigned tagged pointer value accessors to tagged pointer providers, but lldb's tagged pointer code only implemented the unsigned one. This patch adds an emulation of the signed one. The motivation for doing this is that NSNumbers use the signed accessor (they are always signed) and we need to follow that in our summary provider or we will get incorrect values for negative NSNumbers. The data-formatter-objc test file had NSNumber examples (along with lots of other goodies) but the NSNumber values weren't tested. So I also added checks for those values to the test. I also did a quick audit of the other types in that main.m file, and it looks like pretty much all the other values are either intermediates or are tested. Differential Revision: https://reviews.llvm.org/D99694 (cherry picked from commit 4d9039c)
This reverts commit 4d9039c. This is causing the greendragon bots to fail most of the time when running TestNSDictionarySynthetic.py. Reverting until Jim has a chance to look at this on Monday. Running the commands from that test from the command line, it fails 10-13% of the time on my desktop. This is a revert of Jim's changes in https://reviews.llvm.org/D99694 (cherry picked from commit 602ab18)
…inters."" This reverts commit 602ab18. The patch replicated an lldbassert for a certain type of NSNumber for tagged pointers. This really shouldn't be an assert since we don't do anything wrong with these numbers, we just don't print a summary. So this patch changed the lldbassert to a log message in reverting the revert. (cherry picked from commit be0ced0)
|
You might want to include bdfee7d as we have Windows bot for Swift. |
(cherry picked from commit bdfee7d)
Done |
|
@swift-ci please test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There no reason to stop a test case just because we encountered an unsupported NSNumber formatter. Apparently the previous check wasn't reached (the preserved number was rejected before the assert). My previous patch to support negative NSNumbers moved this check earlier and started triggering the assert. That wasn't helpful, so I changed the lldbassert's to data formatter channel log messages.