Lower Backtrace and SourceContext to SPI.
#539
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
grynspan
requested review from
stmontgomery,
dennisweissmann,
briancroom,
SeanROlszewski and
suzannaratcliff
as code owners
|
@swift-ci please test |
grynspan
commented
Jul 16, 2024
| @@ -22,9 +22,13 @@ extension ABIv0 { | |||
| /// The location in source where this issue occurred, if available. | |||
| var sourceLocation: SourceLocation? | |||
|
|
|||
| /// The backtrace where this issue occurred, if available. | |||
| var _backtrace: [Backtrace.Address]? | |||
There was a problem hiding this comment.
This is not formally part of the JSON schema (hence the underscore) but I realized it was missing and should be present here.
`Backtrace` is our implementation of a stack trace type. Xcode 16 uses it to provide richer information about test failures than can be provided by _just_ an instance of `SourceLocation`. The standard library has its own type, `_Backtracing.Backtrace`, but due to our deployment requirements on Apple platforms, we can't use it yet. It would make sense for us to adopt the `Backtrace` type in the standard library as soon as possible. To avoid staging issues in the future, and because our type has not undergone any sort of formal review, I'm lowering it to tools-only SPI. I'm also lowering `SourceContext` because it consists of an instance of `Backtrace` and an instance of `SourceLocation`—but if `Backtrace` is SPI, then the public interface of `SourceContext` is simply a wrapper around `SourceLocation`. This does not eliminate the internal need for the type, but it does make it redundant from an API perspective. If and when we regain `Backtrace` as a public interface, we can look at raising `SourceContext` too.
grynspan
force-pushed
the
jgrynspan/lower-backtrace-to-spi
branch
from
d5c52a6 to
c40ee60
Compare
|
@swift-ci please test |
briancroom
approved these changes
Jul 16, 2024
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.
Backtraceis our implementation of a stack trace type. Xcode 16 uses it to provide richer information about test failures than can be provided by just an instance ofSourceLocation.The standard library has its own type,
_Backtracing.Backtrace, but due to our deployment requirements on Apple platforms, we can't use it yet. It would make sense for us to adopt theBacktracetype in the standard library as soon as possible. To avoid staging issues in the future, and because our type has not undergone any sort of formal review, I'm lowering it to tools-only SPI.I'm also lowering
SourceContextbecause it consists of an instance ofBacktraceand an instance ofSourceLocation—but ifBacktraceis SPI, then the public interface ofSourceContextis simply a wrapper aroundSourceLocation. This does not eliminate the internal need for the type, but it does make it redundant from an API perspective. If and when we regainBacktraceas a public interface, we can look at raisingSourceContexttoo.Checklist: