Better runtime failure messages: 2nd try #26162
Merged
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
…iteral, indicating the failure reason.
The SIL generation for this builtin also changes: instead of generating the cond_fail instructions upfront, let the optimizer generate it, if the operand is a static string literal. In worst case, if the second operand is not a static string literal, the Builtin.condfail is lowered at the end of the optimization pipeline with a default message: "unknown program error".
To display a failure message in the debugger, create a function in the debug info which has the name of the failure message. The debug location of the trap/cond_fail is then wrapped into this function and the function is declared as "inlined". In case the debugger stops at the trap instruction, it displays the inline function, which looks like the failure message. For example: * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0) frame #0: 0x0000000100000cbf a.out`testit3(_:) [inlined] Unexpectedly found nil while unwrapping an Optional value at test.swift:14:11 [opt] 11 12 @inline(never) 13 func testit(_ a: Int?) -> Int { -> 14 return a! 15 } 16 This change is currently not enabled by default, but can be enabled with the option "-Xllvm -enable-trap-debug-info". Enabling this feature needs some changes in lldb. When the lldb part is done, this option can be removed and the feature enabled by default.
For using the improved condfail messages in the stdlib, we need a function, like precondition(), just taking a StaticString instead of a String for the message. The existing (underscored) _precondition is a perfect fit for this, except that it's not printing the location info in debug builds. This change makes _precondition() equivalent to precondition, just taking a StaticString as argument. The other alternative would be to add another variant of precondition, just taking a StaticString. But we already have so many failure functions in Assert.swift, so adapting an existing one seems to be a better solution. This effectively undos a change from 5 years ago which intentionally removed the location info from _precondition (rdar://problem/16958193). But this was at a time where swift was not open source yet. So I think today it's okay to always add location information, even if it's from inside the stdlib. It can be even very useful for expert users for looking up the location the stdlib source.
…nil while unwrapping an optional" failure message. Advantage: the failure message is now also visible in release builds.
Contributor
Author
|
@swift-ci test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Reinstates #25978 + a few fixes in test files.
rdar://problem/51278690
plus fixes for tests: rdar://problem/53116178, rdar://problem/53115905