[SE-0458] Adopt strict memory safety in the standard libraries #78372
Conversation
DougGregor
requested review from
artemcm,
al45tair,
phausler,
ktoso,
zoecarver,
hyp,
egorzhdan,
Xazax-hun,
slavapestov,
a team,
xymus,
AnthonyLatsis,
hborla,
xedin,
tshortli,
ahoppen,
bnbarham,
hamishknight,
rintaro,
beccadax,
ian-twilightcoder and
CodaFi
as code owners
DougGregor
force-pushed
the
strict-safety-stdlib
branch
from
8c91b2a to
12b1db1
Compare
DougGregor
requested review from
etcwilde,
compnerd,
edymtt and
justice-adams-apple
as code owners
| // if it was present on the command line. | ||
| auto &paths = outputPaths.back(); | ||
| if (auto fixitPath = Args.getLastArg(options::OPT_emit_fixits_path)) { | ||
| if (paths.FixItsOutputPath.empty()) { |
There was a problem hiding this comment.
Why this check? The if (auto ... = Args.getLastArg(options::OPT_emit_fixits_path)) should get just the last entry.
There was a problem hiding this comment.
Presumably, the supplementary outputs file could have an entry here, so we're saying that would take precedence over the command-line argument.
stdlib/public/CMakeLists.txt
Outdated
| @@ -18,7 +18,7 @@ endif() | |||
| if(SWIFT_STDLIB_SIL_DEBUGGING) | |||
| list(APPEND SWIFT_RUNTIME_SWIFT_COMPILE_FLAGS "-Xfrontend" "-sil-based-debuginfo") | |||
| endif() | |||
|
|
|||
stdlib/public/CMakeLists.txt
Outdated
| @@ -18,7 +18,7 @@ endif() | |||
| if(SWIFT_STDLIB_SIL_DEBUGGING) | |||
| list(APPEND SWIFT_RUNTIME_SWIFT_COMPILE_FLAGS "-Xfrontend" "-sil-based-debuginfo") | |||
| endif() | |||
|
|
|||
DougGregor
force-pushed
the
strict-safety-stdlib
branch
from
12b1db1 to
617556c
Compare
| @@ -575,7 +575,7 @@ extension _ArrayBuffer { | |||
|
|
|||
| @inlinable @_alwaysEmitIntoClient | |||
| internal func getAssociatedBuffer() -> _ContiguousArrayBuffer<Element>? { | |||
| let getter = unsafeBitCast( | |||
| let getter = unsafe unsafeBitCast( | |||
There was a problem hiding this comment.
We're going to need condfail mitigations in all the fragile functions, right?
There was a problem hiding this comment.
Yeah, I had an idea on that... since we have a swift-syntax tree for the inlined text, I think we can write a visitor that replaces UnsafeExprSyntax nodes with their argument.
There was a problem hiding this comment.
I'm glad my SwiftIfConfig rewrite can be useful :)
Allow a conformance to be "isolated", meaning that it stays in the same isolation domain as the conforming type. Only allow this for global-actor-isolated types. When a conformance is isolated, a nonisolated requirement can be witnessed by a declaration with the same global actor isolation as the enclosing type.
When a protocol conformance somehow depends on an isolated conformance, it must itself be isolated to the same global actor as the conformance on which it depends.
… default With the acceptance of SE-0458, allow the use of unsafe expressions, the @safe and @unsafe attributes, and the `unsafe` effect on the for..in loop in all Swift code. Introduce the `-strict-memory-safety` flag detailed in the proposal to enable strict memory safety checking. This enables a new class of feature, an optional feature (that is *not* upcoming or experimental), and which can be detected via `hasFeature(StrictMemorySafety)`.
Port over the disambiguation code we already had in the new Swift parser to the existing C++ parser for the "unsafe" expression and for..in effect.
DougGregor
force-pushed
the
strict-safety-stdlib
branch
from
61f0793 to
7aac28e
Compare
DougGregor
changed the title
DougGregor
changed the title
|
@swift-ci please smoke test |
|
Note that we have some serious stacking of PRs happening right now. The first five commits are covered by #79645 |
|
@swift-ci please smoke test macOS |
|
@swift-ci please smoke test Windows |
|
@swift-ci please smoke test |
|
Just do double check that this (large) change has no impact on code generation: @swift-ci apple silicon benchmark |
|
@eeckstein There should be zero impact on performance because there's no SIL-level differences for anything related to the strict memory safety checking. I think the results of that performance run look fine, but there are a few's ?'s I don't know how to interpret. |
|
Yes, it's completely fine. The "?" results are noise. If there would be a real impact, the results would look much different. |
With the acceptance of SE-0458 "Opt-In Strict Memory Safety Checking" and its enablement in the compiler, adopt strict memory safety through most of the Standard Library. This involves passing the
-strict-memory-safetyflag to the compiler when building these standard library modules, and adding all of the necessary@safe/@unsafe/unsafeannotations to acknowledge uses of existing unsafe code.There are a lot of
@unsafeandunsafeannotations, hence the large diff. This is, fundamentally, because it's the Swift standard libraries that layer safe abstractions (arrays, dictionaries, strings, etc.) on top of unsafe primitives (raw UTF-8 bytes and such). The annotation here was largely mechanical, driven by compiler Fix-Its with some review and massaging.