PanicAdapter + panic auditing #12
Conversation
| @@ -1819,7 +1822,7 @@ impl<A: DeallocRef> String<A> { | |||
| /// # Panics | |||
| /// | |||
| /// Panics if the starting point or end point do not lie on a [`char`] | |||
| /// boundary, or if they're out of bounds. | |||
| /// boundary, or if they're out of bounds, or if there is an allocation failure | |||
There was a problem hiding this comment.
Previous undocumented panic condition. Splice potentially allocating on drop is a very subtle edge case it's super easy to forget!
|
|
||
| impl<T> UncheckedOptionExt<T> for Option<T> { | ||
| unsafe fn unwrap_unchecked(self) -> T { | ||
| if let Some(t) = self { t } else { unreachable() } |
There was a problem hiding this comment.
I don't know what the justification of all this was; good thing we don't need it anymore!
Ericson2314
force-pushed
the
infallibility
branch
2 times, most recently
from
6eaa40f
to
005f1b5
Compare
|
Huh. Not sure where this test failure is coming from. It shows up after the second commit, so it must have something to do with the first two. (The first alone doesn't test |
|
Thanks for the PR. How does this implementation differ from the removed The test failed for the exact same reason why I removed |
Ericson2314
force-pushed
the
infallibility
branch
from
005f1b5
to
767a37e
Compare
Ericson2314
force-pushed
the
infallibility
branch
from
767a37e
to
39869d7
Compare
Ericson2314
force-pushed
the
infallibility
branch
from
39869d7
to
c20cb95
Compare
|
Closing in favor of |
As done in rust-lang/rust#60703, I make a
PanicAdapterto be the one place we callhandle_alloc_error, and carefully threadError = !everywhere else (which forces the use ofPanicAdapter, something similar, or a magic allocator which truely never fails.The old way is still much better than the status quo, to be clear, but makes it harder to audit code for allocation handling. Any library, anywhere, may call the regular methods instead of the
try_methods, making it hard to rely oncrates.io. The panics are strewn about, and it is hard on the library maintainers to remember what the invariants are.But with this way, all one needs to do is make sure
PanicAdapteris never emitted (check debug symbols perhaps?) and one can be sure that there are no accidental panics by a third party, as the regular methods will be uncallable. (They still can explicitly panic, of course, but that's a lot less likely to be done by accident.)Taking a step back, people that really care about handling all control flow, and existing users of the standard library simply require different interfaces. Anything less than the full
try_sweet, with ways to guard against the use of the "other half", does a disservice to the control-flow-concious crowd. The conventional wisdom is that we cannot satisfy that need without an entire new stdlib; this PR is meant to show that in fact we can; the interfaces grows faster than the implementation so it's sustainable.Less this all seem academic, I will point out some cases of tricky allocation failure that I was forced by the types to catch below.