Don't allow unwinding from Drop impls

This RFC proposes to change this behavior to always abort if a panic attempts to escape a drop, even if this drop was invoked as part of normal function execution (as opposed to unwinding which already aborts with a double-panic).
Previous discussion: https://github.com/rust-lang/lang-team/issues/97
Rendered

I think it would also be good to add an explicit section to the RFC about how users will find out about the change in behavior, just to document that going in - for example, release blog post and compat note would be a good start in my eyes, but we could also consider some softer change (e.g., adjusting the panic logging to say "this will abort starting in X release) to phase this in. AIUI, there's not really much static checking we can do, so that's probably about our limit...

I'm a huge fan of this proposal. When writing the rustonomicon this was one of the few major details of rust I couldn't really get a straight answer from anyone about. From my perspective it existed in the same grey area as unwinding through FFI, where we just lacked institutional expertise on the subtleties of unwinding corner cases and had punted on it.
This is why I never really adequately documented it. The current behaviour is very subtle and not something even stdlib devs ever had institutional knowledge or consideration of.
Writing exception-safe unsafe code is already enough of a headache, partially because it comes up rarely enough that it's hard to stay actively vigilant for. I think like, BinaryHeap is one of the only places in liballoc that has to do some explicit exception-safety fuckery because it needs to invoke the user-defined comparator repeatedly while doing mutations that put it in a transiently invalid state. All those stars lining up is actually surprisingly rare! Most of the time there are distinct lookup, allocate, then modify phases; so code is trivially exception-safe (assuming your own code doesn't panic, which collections generally only do during the lookup and allocations, not the final modification). Having less places where this rare problem can surprisingly come up, especially from user code, seems like a great thing.
We've never had a good story about this situation and at this point I think we have ample evidence it should just be banished to the shadow realm for being more trouble than it's worth. Especially since the stdlib already has the established precedent of Buf and File both just swallowing errors on Drop. Those cases were going to be my one hand-wring for this change, so it turns out I have nothing to complain about! (My memory was that they panicked, but honestly I never really gave it much consideration).

I'd like to rehash my earlier arguments against this change. I'd like to begin with some statements that I think everyone here agrees with:

Disregarding code size, this change only assists unsafe code. All safe code remains safe in the presence of unwinding panics.
Authors of some applications, such as certain web servers, would strongly prefer that threads unwind on critical failure rather than abort the entire process.
Some real-world Rust code can panic on drop today. (For instance, a Drop impl that calls println!() can panic if it fails to write.)
Today, a normally-unwinding panic from a Drop impl aborts the process if and only if either -C panic=abort is set, or the thread is already unwinding. With this change, a panic from a Drop impl will abort unconditionally.
As a consequence, some code that used to unwind the thread will now abort the process, to the (perhaps minor) detriment of certain web servers and some other applications.

Thus, I believe that since this is a silent change that will harm some current use cases, we should really show that the benefit of this change is greater than the cost, and that there are neither more beneficial nor less costly alternatives. I argue that such an alternative exists: keep panic-in-drop=unwind as the default, and provide better tools for writing drop-sensitive unsafe code.
Currently, the RFC twice compares this change to C++11's noexcept destructors, and only briefly mentions the differences in the "Prior Art" section. However, these differences are very important. In C++, each function parameter and local variable runs its destructor at the end of its scope. There is no way to prevent this, short of performing certain hacks with a union. There is also no way to run the destructor explicitly; even a move operation creates a new value and leaves a sentinel which must be destructed. But in Rust, destruction is determined by ownership semantics. If we pass a variable into a function parameter by value, the function (or data structure it is attached to) will become responsible for dropping it. This allows us to explicitly drop any value at any point in time, by passing it to the aptly-named drop() function.
The ability of Rust code to explicitly control dropping is quite relevant here. In C++, destruction is always invisible and uncontrollable (except if a value is placed into an std::optional which is reset), and so it makes a lot of sense not to mandate exception-safety surrounding it. In Rust, we can make drops clearly visible in unsafe code by using drop() and similar constructions, so the rationale doesn't apply nearly as absolutely.
The problem is, unsafe code today can easily implicitly drop values without realizing there's an issue; just look at the catch_unwind footgun (rust-lang/rust#86027). I believe that the best and most direct way to mitigate this is to provide better tools, both in the compiler and in the standard library, to catch these footguns instead of leaving them for the unsafe code's author to figure out. Importantly, we should clearly recommend using these tools in our docs and our books. One crude but effective tool, from a suggestion by @danielhenrymantilla, would be an allow-by-default lint that triggers on every variable that implicitly calls Drop::drop(), and suggests explicitly drop()ping the variable. We could advise library authors to use this on any panic-sensitive code that handles trait objects or user data types.

There are a few different variations on how the panic-in-drop option is to be handled. To summarize my position on each:

-Z panic-in-drop=unwind by default, -Z panic-in-drop=abort allowed: The current status quo. Acceptable, given additional tools to help unsafe code authors.
-C panic-in-drop=unwind by default, -C panic-in-drop=abort allowed: My preferred option, given additional tools. Users could manually choose panic-in-drop=abort for the improved code size and compile times, without needing the fully-fledged panic=abort.
-Z panic-in-drop=abort by default, -Z panic-in-drop=unwind allowed: This proposal, which I am against. It provides the code-size and compile-time improvements, but introduces a silent change that restricts some use cases with no stable workaround.
-C panic-in-drop=abort by default, -C panic-in-drop=unwind allowed: I am strongly against this. It would make unwinding drops possible, but it would fracture the package ecosystem: new unsafe container libraries could easily ignore support for the now-special panic-on-drop=unwind configuration, making the option very dangerous to use.

I would like to echo that the RFC as currently written puts too much weight on the code size / compilation benefits of the change. These are a strong argument for making the option available, but distract from the correctness argument for making it mandatory. Presenting this in the main text which is concerned with justifying changing the semantics of existing code serves to distinctly weaken the justification, since the argued position is not necessary for a primary argued benefit.
If the change is to be made, it should be justified on the unwind safety benefits alone, and the other benefits are just a very convenient bonus.
There is additionally a third undiscussed option: panic-in-drop=catch. Effectively, this automatically converts types to the recommended panic-in-drop=abort behavior of attempting as much cleanup as possible but swallowing the error and continuing on if something goes wrong. Of course, this is not without its own annoying problems (e.g. a field relying on the parent destructor to put it back into a state that is safe to drop... though imho that should definitely be using ManuallyDrop already anyway), but it does still have the distinct advantage of providing the nounwind benefit while avoiding
I also need to point out what I think should be addressed, or at least acknowledged as a drawback: it is impossible to drop a structure where dropping its fields may panic without risking a panic-in-drop=abort. Even if you use ManuallyDrop to drop fields within your drop impl rather than after, the field's drop glue will cause an abort, so if you have legacy code that e.g. calls eprintln! in its destructor[^1], you have no recourse other than to forget to drop that field and leak its resources.
[^1]: ... I have some code to go update to be more resilient against double panics.
A final really silly option: add ptr::drop_in_place_maybe_unwind. This would explicitly generate and call panic-in-drop=unwind style drop glue, allowing you to lift panickingaborting destructors into a panicking close automatically.

If the compiler had some sort of #[no_panic] support that could be used for drop impls, I would be less worried about a change along these lines, because it would be at least somewhat possible to verify that drops don't cause an abort in normal execution.

I'll just note that the performance and code size changes are pretty inconsequential, for a break in backwards compatibility. 3-5% decrease in code size is irrelevant for all but the most resource-constrained applications, which will likely use panic=abort anyway.
Also, while the change would possibly simplify the correctness of unsafe code, it would compromise the correctness of the application at large, which may rely on properly handling Drop panics. This is a big issue, both because the application is much harder to audit due to its size and lack of compiler-enforced panic boundaries, and because the application is usually written by less experienced users, including total newbies, while unsafe code is relegated to the experts.
The change in behaviour of Drop would be a breaking change, and a very insidious one, since it is entirely silent and hard to audit. That's the worst kind of breakage that can be introduced. It is also well-known that Rust currently allow panic in Drop, so many crates may rely on it. scopeguard is one mentioned example, Another example is the drop_bomb crate, which is used in 53 dependencies, including rust-analyzer. I'm sure there are Drop impls in the wild which use the same pattern without explicitly using those dependencies.

An attribute that specific unsafe types can put on their own drop impl if they need to be absolutely sure that no unwind can escape their drop is one thing. This is too much.

I feel this is a throwing baby with the bathwater situation. I'd prefer exploration of alternative designs. For example:

Add ability to forbid all panics in a function or a block of code. For unsafe code a panic in Drop is just one of many hazards. There could be unexpected panics lurking in Deref and overloaded operators. unsafe needs to be aware of all of them, so this solution is insufficient. OTOH forbidding unwinding in all the other unobvious places would equal to effectively removing panic from Rust.

catch_unwind is a gotcha, but it seems to be a very unique case. There is no other panic-catching function, so there is no other place in Rust where code clearly doesn't want a panic but may still get one. I feel that a solution narrowly targeted specifically at catch_unwind would be sufficient to solve this gotcha, e.g. forbid panic in drop only in types used with catch_unwind (and dyn if necessary) or mark only callers of catch_unwind with panic=abort

For applications concerned about code size overhead there is panic=abort already. If a more granular solution is needed, this could be offered with an explicit function attribute.

I would like to echo that the RFC as currently written puts too much weight on the code size / compilation benefits of the change. These are a strong argument for making the option available, but distract from the correctness argument for making it mandatory. Presenting this in the main text which is concerned with justifying changing the semantics of existing code serves to distinctly weaken the justification, since the argued position is not necessary for a primary argued benefit.

I personally find the code size argument more convincing since it is backed by hard numbers which show the improvement. The exception safety argument is inherently subjective since it can just be countered by saying "well it's your fault for writing buggy unsafe code". It's not impossible to write exception-safe Rust code, just difficult.

Also, while the change would possibly simplify the correctness of unsafe code, it would compromise the correctness of the application at large, which may rely on properly handling Drop panics. This is a big issue, both because the application is much harder to audit due to its size and lack of compiler-enforced panic boundaries, and because the application is usually written by less experienced users, including total newbies, while unsafe code is relegated to the experts.

I find this argument quite strange. Does any existing code actually rely on panics for normal operation? In general, panics will only happen in the case of buggy code, when an unexpected error occurs. Additionally, the proportion of code executed by Drop impls is tiny compared to non-drop code: assuming an even distribution of potential unexpected panics in a codebase, they will almost always come from non-drop code.

There's additionally another intriguing middle ground which should be listed as an alternative: drop glue / drop_in_place can still unwind, and a field's drop glue unwinding continues to unwind and drop other fields (siblings and of parent containing types), but unwind is implicitly caught and turned into an abort only when the unwind exits from the drop glue stack into a calling function.

This is something that has been suggested a lot. If I understand correctly, what you are suggesting is that unwinding out of a drop aborts only for implicit drops but not explicit ones. The problem is: if you already know which drop calls are going to potentially panic then you could just convert that code to use a close method like I suggested in the example. I don't see much benefit in adding compiler support for something which can easily be done in library code.

In order to justify changing the default, the RFC should justify the claim that RAII drop glue is an issue, and not just explicit drop_in_place calls unwinding.

The RAII drop glue calls drop_in_place, so making that function nounwind will affect both explicit and implicit drops.

I would much rather see (and am drafting) a proposal providing APIs for unsafe code to control unwinds more carefully; namely some combination of:

The problem with these is that none of them give the same code size & compilation time benefits as the approach proposed in this RFC. This is something that I feel quite strongly about: a 10% speedup in compilation time on a crate like syn is huge.

I personally find the code size argument more convincing since it is backed by hard numbers which show the improvement. The exception safety argument is inherently subjective since it can just be countered by saying "well it's your fault for writing buggy unsafe code". It's not impossible to write exception-safe Rust code, just difficult.

Given that the Rust standard library (ostensibly a codebase with a much higher level of rigor and review than most crates) has gotten this wrong on several occasions, I'm doubtful that most developers would be able to disarm this footgun, or even recognize that this exists.

Does any existing code actually rely on panics for normal operation? In general, panics will only happen in the case of buggy code, when an unexpected error occurs.

Panics may be used to implement non-local exception-based control flow, even though it is clearly not an intended usage.
But to the point, while I doubt that panics could occur during normal operation of most non-buggy code, the correctness argument is about the graceful handling of bugs. Code may rely on being able to catch the panics for correctness. A long-running web server, or an embedded device, or a security-critical firmware, or an OS kernel absolutely should not panic. End-users may rely on being able to catch any possible panics with catch_unwind, and currently this includes single panics occurring in Drop glue. Note that the panic strategy is chosen by the end-user during compilation. If one sets -C panic=unwind, they can reasonably expect that they will be able to handle panics.
With the proposed changes, a relatively benign panic for some rare edge case will be escalated to a total crash, which can lead to DoS or worse.

Additionally, the proportion of code executed by Drop impls is tiny compared to non-drop code: assuming an even distribution of potential unexpected panics in a codebase, they will almost always come from non-drop code.

The argument about distribution of panics is quite weird, I don't know what to make of it. Since Drop must deal with disposing potentially fallible resources, I find it likely that Drop impls on average have more panics, but this is also a baseless assumption.

I personally find the code size argument more convincing since it is backed by hard numbers which show the improvement.

But the code size can be achieved by opt-in or opt-out behavior. It is not a justification for making panic-in-drop=abort the only option. It can serve as a supporting argument to making the change, but it cannot be the primary argument, because it does not justify making this breaking change to behavior.

I find this argument quite strange. Does any existing code actually rely on panics for normal operation?

Yes. For example, rust-analyzer uses unwinds for cancellation. Unwinding is the semantically correct way to handle cancellation, and is essentially what cancelling an async function and dropping the witness table looks like (just without thread::panicking()).
This is not unique to just rust-analyzer; this is a design choice of salsa.
It has generally been understood that while you SHOULD NOT require unwinding support in a library (and MUST support unwinding), you MAY rely on unwinding being available if you control the application compilation.
I agree that likely no code is deliberately relying on unwinding out of drop for application correctness, but it is the case that long-running programs intend to catch any task unwind, and turning unwinds that would have been recovered from into aborts is adding a new DoS vector to such servers.

This is something that has been suggested a lot. If I understand correctly, what you are suggesting is that unwinding out of a drop aborts only for implicit drops but not explicit ones. The problem is: if you already know which drop calls are going to potentially panic then you could just convert that code to use a close method like I suggested in the example. I don't see much benefit in adding compiler support for something which can easily be done in library code.

Then I can take this argument and apply it unchanged to making drop_in_place nounwind as well! You already know where things are going to potentially panic (every drop_in_place call) and can convert the code to use a close method.
Unless I'm sorely mistaken, the justification for making this a mandatory breaking change without opt-out is that it is too difficult to know which drop calls are going to potentially panic, and the proposed resolution is to make no drop calls panic. I fail to see how this alternative fails at the goal, as every point that can unwind is clearly marked. And if you do want the behavior where drop_in_place aborts unwinds as well, drop_in_place has a reminder on it to "remember that this can unwind from panicking drop glue! If this is undesirable, wrap the call in a drop_unwind or call drop_in_place_no_unwind instead."
I'd even be happy if we took the usual "safer by default" approach (as with copy/copy_nonoverlapping, sort/sort_unstable) and made drop_in_place abort unwinds but provided (and drop glue used) drop_in_place_allow_unwinds. The thesis I am defending is to provide safeguards but with the choice and tools to allow unwinds.

The problem with these is that none of them give the same code size & compilation time benefits as the approach proposed in this RFC. This is something that I feel quite strongly about: a 10% speedup in compilation time on a crate like syn is huge.

I will again also reiterate that the compile-time and code-size improvements are not an argument to make this mandatory. They are a very good argument to make it an option. They may even support making this the default. They would definitely be an argument against adding panic-in-drop=unwind if the existing behavior was panic-in-drop=abort.
Swift's lack of deinit throws is reasonable prior art to point to for not providing fallible destruction. However, Swift has a large advantage here: they don't have any unwinding. Your only choice is throws or abort. This means that you have a built-in guarantee that a Swift destructor doesn't contain an uncaught exception/unwind. In fact, in Swift, indexing an Array out of bounds aborts. (Most collections return an optional value from subscripts, but Array doesn't.) Basically any developer error which would panic! in Rust is an abort in Swift.
If anything, this is an argument in support of -Cpanic=abort becoming the default instead of -Cpanic=unwind. I would support such a change. (This may run into similar issues around encouraging crates which don't support unwinding, but tests continuing to be -Cpanic=unwind I think serves as a strong force against this.)

In order to justify changing the default, the RFC should justify the claim that RAII drop glue is an issue, and not just explicit drop_in_place calls unwinding.

The RAII drop glue calls drop_in_place, so making that function nounwind will affect both explicit and implicit drops.

Yes, but I'm not arguing that point. It is a perfectly viable strategy for the unwind-to-abort shim to be added in the drop glue rather than in drop_in_place.

Given that the Rust standard library (ostensibly a codebase with a much higher level of rigor and review than most crates) has gotten this wrong on several occasions, I'm doubtful that most developers would be able to disarm this footgun, or even recognize that this exists.

Nobody (including std devs) really understood the issue of a caught unwind's drop unwinding until recently. (This discussion started almost immediately after it was discovered.) We can attack this with documentation on catch_unwind and any other APIs for managing unwinds.
Other than this specific case, std has generally had no issue with unwind safety, because it is something you so rarely have to think about that std basically only encounters it for

In order to justify changing the default, the RFC should justify the claim that RAII drop glue is an issue, and not just explicit drop_in_place calls unwinding. I would much rather see (and am drafting) a proposal providing APIs for unsafe code to control unwinds more carefully

I wrote up an outline for something like this in an IRLO thread a while back; the idea is to have a catch_unwind_v2<F, R>(...) -> UnwindResult<R>, where UnwindResult is #[must_use] and has many of the methods you listed, acting as a sort of by-value builder API. To summarize what I wrote there:
use std::{any::Any, panic::UnwindSafe};

pub fn catch_unwind_v2<F: FnOnce() -> R + UnwindSafe, R>(f: F) -> UnwindResult<R>;

#[must_use]
pub struct UnwindResult<T> { ... }

impl<T> UnwindResult<T> {
    // Unwinding functions
    pub fn into_result(self) -> Result<T, Box<dyn Any + Send>>;
    pub fn drop_err(self) -> Option<T>;
    pub fn resume_unwind(self) -> T;

    // Unwind-free functions
    pub fn inspect_err<F>(self, f: F) -> UnwindResult<T>
    where
        F: FnOnce(&(dyn Any + Send + 'static));

    pub fn inspect_err_mut<F>(mut self, f: F) -> UnwindResult<T>
    where
        F: FnOnce(&mut (dyn Any + Send + 'static));

    pub fn forget_err(self) -> Option<T>;
    pub fn unwrap_or_abort(self) -> T;
    pub fn try_drop_err(self) -> Result<T, UnwindResult<()>>;
    pub fn drop_err_or_forget(self) -> Option<T>;
    pub fn drop_err_or_abort(self) -> Option<T>;

    // Add `new()`, `map()`, etc. as desired
}

I've been doing some surveying of real-world catch_unwind usage (via GitHub Code Search) to evaluate this; I'll probably post my findings in a few days.

After a bit of research, I wanna say I don't particularly find the comparisons to C++ noexcept particularly compelling. In C++, the basic exception guarantee is as follows:

If the function throws an exception, the program is in a valid state. No resources are leaked, and all objects' invariants are intact.

Rust code provides neither of these guarantees, even in idiomatic code. (It is only better than having no exception guarantee that memory corruption iins not allowed). Leak amplification is used throughout  the Rust stdlib (unlike C++).
In addition, unwinds do not require object invariants to be preserved, which is the entire reason why UnwindSafe is a thing, but issues like rust-lang/rust#89832 aren't really considered worth fixing. Observing broken object invariants in Rust is safe, for better or for worse.
Given that objects with throwing destructors were already forbidden in STL containers, the change in C++ likely impacted a lot less code than these proposed Rust changes. And I think given the difference in semantics for unwinding vs exceptions, this change is too big a hammer.

Yeah, UnwindSafe is kinda "fake" in the sense that it doesn't relate to unsafe at all.

The RFC text (in the Downsides) ought IMO to discuss the "drop bomb" pattern, sometimes used to simulate linear types - or rather, to avoid shipping code which treats a particular type as nonlinear.  E.g. https://github.com/rust-lang/rust/pull/78802#issuecomment-1177948974
This is relevant here, because drop bombs precisely involve deliberately panicking inside drop, in circumstances where the author would probably have made very different design choices if they expected that such a panic would be an immediately irrecoverable abort.

Drop bombs are usually being used to cover up for things you would want to be compile-time errors, so it's not at all clear to me that you would care about whether the bomb unwinds or aborts -- if linear (true must-use) type lovers had their druthers, the program wouldn't have even been runnable!

"you already can't recover from lock poisoning" was obviously a gap in the design, and closing that gap is already in progress: https://github.com/rust-lang/rust/issues/96469
So we're actually already working to improve the DoS situation, and the RFC as written would worsen things.
There's three alternatives given, including "opt-in to unwinding", but there isn't an alternative given for "opt-out of unwinding". Since particular unsafe code is the code that can go wrong with a panic in drop (not all unsafe code does), why don't we put the burden on the particular instance of unwind sensitive unsafe code to opt in to having a drop that will abort if an unwind tries to escape from it? Call it #[about_on_unwind_escape] or whatever name. That seems the least disruptive to existing code, while allowing unsafe code the tools it needs to be correct.

@Amanieu We discussed this in today's @rust-lang/lang meeting.
We felt like all of the arguments have been made at this point, and now we need to make a call between possibilities.
However, before we do that, there are a few changes we'd like to see:

The correctness argument should be at the forefront, as that's the primary justification for doing this. Performance is a bonus, and not a reason to change the default.
There should be some kind of hook that runs when aborting, to give code an opportunity to do cleanups. (The same hook should run on all panics with panic=abort.)

The correctness argument should be at the forefront, as that's the primary justification for doing this.

Once the correctness argument is at the forefront, I think it would be great for the RFC to also consider (or at least mention) alternatives that would aid the user in writing correct code, such as:

a less error-prone catch_unwind API.
a lint against any implicit method calls.
some form of #[panics(never)] attribute.

I'm no expert on the compiler, but I feel like (2) should be reasonably easy to implement.
Implementing (3) in a useful manner would likely require a lot of additional design work[^1].
[^1]: Last time I though about this I ended up with multiple additional qualifiers like #[panics(never(verify_unstable)], #[panics(never(assume_unsafe)] or #[panics(never(type_dependent)]. Some could be safely combined (like #[panics(never(verify_unstable,type_dependent)]) but others should be linted against (like #[panics(never(assume_unsafe,type_dependent)] unless the annotated function is unsafe). And those qualifiers still don't cover all generic use cases.

There should be some kind of hook that runs when aborting, to give code an opportunity to do cleanups. (The same hook should run on all panics with panic=abort.)

This already exists: an unwind guard will call the panic handler with a PanicInfo that return false for can_unwind (#92988). std's panic handler will call the panic hook as normal but then abort instead of unwinding.

a less error-prone catch_unwind API.

FWIW, the catch_unwind API problem is kind of already paliated with https://github.com/rust-lang/rust/pull/99032.
Then, with @LegionMammal978's proposal, and potentially with also something like ::unwind_safe's API, we'll be able to offer rich and more powerful APIs for future Rust code (I'm thus looking forward to your counter-proposal, @CAD97!), without needing to break language semantics.

I'll reiterate that the tool that would help catch unwind-related footguns,

(regarding which the drop ones have taken a gigantic attention with this proposal, but .-derefs, {:?}-formatting, + and-the-like operations, .awaits, etc., are other sneaky places where unwinds can happen)

is to have an allow-by-default lint against API usage that may unwind (or implicit drops if that is still deemed to be that devilish). This is definitely information accessible to the MIR output, so such a lint ought not to be that difficult to write.
So, to go back to:

Handling objects that panic in their drop implementations is incredibly complex, and as noted, even the standard library has gotten it wrong. I think it's reasonable for us to evaluate whether we expect libraries to handle this, or whether we want to proscribe it entirely so that libraries don't have to handle it.

I think that before we ask that question, especially considering breaking semantics of the language, that we should first give a try to these helpful tools.
In summary:

merging https://github.com/rust-lang/rust/pull/99032 to get catch_unwind's footgun tackled,
and maybe also implementing @LegionMammal978's and @CAD97's proposal for an even better API in that fashion;
featuring a lint or some other tool to help identify unwind points;
as well as blessing scopeguard and/or unwind_safe patterns to be part of std, since with those tools writing unwind-safe code is quite easy, to be honest (thanks to nested unwinds aborting).

Once all this is done, if the situation is still deemed problematic w.r.t. unwinds in implicit drops or with other sneaky ops, then a PR to tackle implicit drops (but no other sneaky ops, and also the innocent explicit drop bystander), only then, imho, ought this removal of unwinds in drop glue be considered.

This already exists: an unwind guard will call the panic handler with a PanicInfo that return false for can_unwind (#92988). std's panic handler will call the panic hook as normal but then abort instead of unwinding.

I may be overlooking it, but is there an explanation of that in the RFC? It'd help to make sure people know what options they'd have.

One thing that popped up while writing my counterproposal: would forbidding unwinding from Drop::drop be able to subsume the panic-in-unwind immediate abort? At least internally it looks like this behavior has not been formally guaranteed.
This sounds extremely scary, but a strong mitigating factor is that existing unwind-to-abort shims still work, because they work by unwinding inside of a destructor. I think it at least reasonably possible that nobody has written code of the form
impl Drop for UnwindsAreAborts {
    fn drop(&mut self) {
        // if unwinding, the panic aborts
        // if not unwinding, catch the unwind and continue
        catch_unwind(|| panic!());
    }
}

or
if panicking() {
    panic!();
    // that aborted, so we can do whatever
    unreachable_unchecked();
}

If we lose the ability to recover from panics escaping drops, but gain the ability to recover from neatly nested panics within drops, even while already unwinding, then it's not just a loss in abort-resilience anymore, but a tradeoff.
Unfortunately, for the case of foreign unwinds, we still require an immediate abort, or else it is UB at the ABI level, as on e.g. the Itanium psABI it is undefined behavior to nest foreign unwinds from different runtimes. (Disclaimer: I do not know what is required to support nested unwinds within a given unwind provider.)

One thing that popped up while writing my counterproposal: would forbidding unwinding from Drop::drop be able to subsume the panic-in-unwind immediate abort? At least internally it looks like this behavior has not been formally guaranteed.

This is mostly orthogonal to this RFC: it's just a matter of removing the panic count and instead relying on the abort guards to catch double panics.
One issue is that std::thread::panicking only checks the panic count but is unable to tell whether a Drop impl is being executed as part of normal execution or is invoked from an unwind landing pad:
fn foo() {
    defer! {
        catch_unwind(|| {
            // std::thread::panicking() returns true here.
        });
    }
    panic!();
}

I am personally not a big fan of the panicking function and would like to see it deprecated, but unfortunately it is quite widely used.

Unfortunately, for the case of foreign unwinds, we still require an immediate abort, or else it is UB at the ABI level, as on e.g. the Itanium psABI it is undefined behavior to nest foreign unwinds from different runtimes. (Disclaimer: I do not know what is required to support nested unwinds within a given unwind provider.)

Although it is technically UB in the ABI, here's what happens in practice:

In C++: catching a Rust panic while a C++ exception is active[^1] aborts (calls std::terminate to be precise).
In C++: catching a C++ exception while a Rust panic is active just works.
In Rust: catching a C++ exception just works, even if a Rust panic is active.
In Rust: catching a Rust panic just works, even if a C++ exception is active.

[^1]; "Active" here means that we are in a catch block for this exception or we are in a destructor that is being run by the unwinder. Note that Rust doesn't have catch blocks so only the latter applies in Rust.

[Allowing contained panics during an unwind] is mostly orthogonal to this RFC

While implementation-wise I agree that this is orthogonal, it is imho strictly-happens-after this RFC, because of the current reliance[^1] on panic-in-unwind producing an abort for soundness.
[^1]: Note to self: replace any instances of this I see with panic!(); abort();. Perhaps another use case for panic_abort!?
Because of this, supporting such is a supporting argument for requiring unwind-in-drop=abort. I would personally predicate accepting this RFC on also migrating to allowing neatly nested unwinds.
The reason for this is that the change is then actually a net increase in robustness against panics. It means that unwinding through otherwise normal code is no longer a potential abort; it makes relying on unwinding better. Currently, using unwinds for control flow is strongly discouraged, because it has the potential to abort even if you control -Cpanic=unwind. Applications can and do rely on unwinding for correctness, however[^2], so improving the resilience of doing so at the cost of unwinds aborting when escaping Drop::drop (and they would've been forced to abort if we were unwinding) starts to seem like a reasonable trade-off.
[^2]: The previous example is salsa implementing synchronous cancellation via unwinds. I argue that unwinding is in fact the correct way to handle cancellation; async cancellation is also identical to an unwind except for the fact that the unwind is stackless/reified, so std::thread::panicking() returns false and starting a real unwind doesn't immediately abort (without this RFC).
I'm one of the most vocal critics of this RFC, but allowing neatly nested panics has made me significantly less resolute in my position. I'm seriously considering adding a more fleshed out than usual alternative in my RFC for "hey, this is an alternative I don't have a concrete rationale to prefer between" for the world where we allow nested unwinds and make Drop::drop abort escaping unwinds.
Combining both resolves my primary concern with either individually, and only leaves the implicit concern/risk of breaking users relying on ossified but formally nonguaranteed behavior (with a silent upgrade to UB).

I've updated the RFC to try to capture all of the feedback and proposed alternatives in this thread. However there has been a lot of discussion so I may have missed something, do let me know.

Well, someone may have suggested "Allow individual Drop impls to opt-in to unwinding", but I suggested roughly the exact opposite: an attribute for Drops that need to opt-in to aborting. With the ability to opt-in to an abort-on-panic for a specific drop, particular unsafe code can guard itself, while most code can continue to unwind without trouble.

I've found at least some current FFI code that intentionally relies on calling resume_unwind() within Drop::drop(). In openssl v0.10.41 (and its fork boring v2.0.0), the associated functions {EcKey, Rsa, PKey}::private_key_from_pem_callback() and PKey::private_key_from_pkcs8_callback() all go through a wrapper invoke_passwd_cb(). This wrapper calls catch_unwind(), storing the payload in the CallbackState. The CallbackState then calls resume_unwind() once it is dropped (source):
pub struct CallbackState<F> {
    /// The user callback. Taken out of the `Option` when called.
    cb: Option<F>,
    /// If the callback panics, we place the panic object here, to be re-thrown once OpenSSL
    /// returns.
    panic: Option<Box<dyn Any + Send + 'static>>,
}

/* ... */

impl<F> Drop for CallbackState<F> {
    fn drop(&mut self) {
        if let Some(panic) = self.panic.take() {
            panic::resume_unwind(panic);
        }
    }
}

This is essentially implementing what I described as "suspend_unwind"; the unwind is caught/suspended and then automatically rethrown/resumed when dropped. This is done for a Rust -> C -> Rust sandwich, in order to propagate a panic over the C code safely. The use of CallbackState is quite similar to std's use of union Data with intrinsics::r#try.

Additionally, a short section about the interaction of pinned memory and drops/unwinds would be useful; this provides motivation for why (in the case of a partial drop failure) we must continue dropping fields.

I'm not sure what you mean by this, could you explain? I'm not sure what pinned memory has to do with this.

I still fall down on the side of preferring allowing unwinds from drop so long as they are properly nested, but this position is significantly less strongly held than before.

Could you clarify what you mean by "properly nested"? If it's just about allowing catch_unwind to be used within a Drop even if it is called during unwinding then this is explicitly allowed by the RFC (though it doesn't work at the moment due to the panic counter):

Note that it is still possible to catch panics inside a Drop impl using catch_unwind. It's when the panic attempts to unwind out of the drop function that the process is aborted.

To provide an extra data point, I've found some widespread FFI code that intentionally relies on calling resume_unwind() within Drop::drop().

That's very interesting, I wasn't aware that there was any code relying on this. I still think that this is a code smell and should be changed to call resume_unwind outside of a Drop.

I'm not sure what you mean by this, could you explain? I'm not sure what pinned memory has to do with this.

Basically, this is the rationale for drop glue dropping fields even after one field's drop glue panicked, rather than leaking the remaining fields.
e.g. drop_in_place<[T]> where T: !Unpin must call Drop::drop on all slice elements before returning (even if via unwind) in order to respect the pinning guarantee of the slice elements not being deallocated before Drop::drop is called.
(An interesting alternative-alternative to allowing drop_in_place unwinds is to add a try_drop_in_place<T> which leaks all drop glue after the first unwind and is unsound to call if T has been pinned. This could also potentially reify the unwind to a Result / do a catch_unwind, though it would likely be marginally better if it used a resume-on-drop captured unwind than a drop-on-drop.)

Could you clarify what you mean by "properly nested"? If it's just about allowing catch_unwind to be used within a `Drop| even if it is called during unwinding then this is explicitly allowed by the RFC (though it doesn't work at the moment due to the panic counter)

Since the wordage here says "it is still possible," it does not change the pseudo-maybe-guarantee that a drop during a panic unwind will immediately abort. I'd like a future possibilities note here about relaxing that behavior to something along the lines of

A panic will immediately abort if started during an unwind and no new catch_unwind has been entered during the unwind; no unwinding occurs if no covering catch_unwind exists.

I hold that we do need to preserve immediate aborts without unwinding for the case of ill-nested panics, as otherwise code in the structure of defer! { defer! { ub() } panic!() } panic!() would become UB rather than an abort. This shape can reasonably occur when code using panics to prevent invariants are nested.
(I also hold that if we make either change to allow well-nested unwinds or to forbid unwinding out of Drop::drop, they need to happen together. Both serve to mitigate the primary downside of the other.)

As another data point, I found another bug caused by panics in drops, this time in hashbrown. The PR to fix it is up: https://github.com/rust-lang/hashbrown/pull/348

I'm surprised this somehow hasn't made it into this thread yet.
rustc relies on panic-in-drop=unwind.
https://github.com/rust-lang/rust/blob/ada80a13b97a01176a1660453060e296a72cf1bb/compiler/rustc_errors/src/diagnostic_builder.rs#L554-L576
/// Destructor bomb - a `DiagnosticBuilder` must be either emitted or cancelled
/// or we emit a bug.
impl Drop for DiagnosticBuilderInner<'_> {
    fn drop(&mut self) {
        match self.state {
            // No `.emit()` or `.cancel()` calls.
            DiagnosticBuilderState::Emittable(handler) => {
                if !panicking() {
                    handler.emit_diagnostic(&mut Diagnostic::new(
                        Level::Bug,
                        DiagnosticMessage::Str(
                            "the following error was constructed but not emitted".to_string(),
                        ),
                    ));
                    handler.emit_diagnostic(&mut self.diagnostic);
                    panic!();
                }
            }
            // `.emit()` was previously called, or maybe we're during `.cancel()`.
            DiagnosticBuilderState::AlreadyEmittedOrDuringCancellation => {}
        }
    }
}

If a diagnostic is constructed but then dropped without being .emit()ted or cancel()ed, then rustc wants to automatically bug!() and unwind to the ICE handler.
Perhaps unwinding isn't necessary here; we emit a bug-level diagnostic which will already be reported as an ICE. It wasn't strictly necessary when this was added in 2015. But if anything, I take this as evidence that triggering an unwind from Drop is specifically desired; the unwind is deliberately used for (programmer error caused) control flow here.

To me this specific example looks like exactly the thing that would be quite straightforward to convert to an abort without any degradation in functionality.

Another bug caused by code incorrectly handling unwinding out of a drop: https://github.com/rust-lang/rust/issues/101628

While we can get rid of the drain-on-drop behavior in the various DrainFilter impls since they're unstable I have noticed that vec::Splice also has drain-on-drop behavior that can evaluate user code.

The input iterator replace_with is only consumed when the Splice value is dropped.

So with this RFC uses of splice that previously panicked would be turned into aborts.

We just discussed the issue of vec::Splice in the libs meeting. We feel that it is unfortunate that this iterator executes user code on drop, but on its own it should not block this RFC.

This is a sweeping change that would make Rust more difficult to justify in contexts where high availability is a requirement. I feel it is out of character for Rust to mandate the usage of abort here. While panic-in-drop=unwind would remain an option, my understanding is one would have to use the nightly compiler to get that back. High reliability contexts also generally prefer to avoid using the nightly compiler. For example, this could pose issues to the Ferrocene project, which is an effort to get Rust certified for use in safety sensitive spaces.
I would soften my stance on this significantly if panic-in-drop=unwind were made available to stable channel users. However in that scenario Rust crates would still need to describe which panic-in-drop modes they support. Perhaps they should be given the ability to compile_error! if a mode is being used which they don't support.
panic-in-drop=abort also harms observability of the code in question. This is something the RFC already mentions, so I won't belabor the point but that is something I consider to be a significant concern.

@Xaeroxe

This is a sweeping change that would make Rust more difficult to justify in contexts where high availability is a requirement

Can you substantiate this with an example? Currently, panicking in a destructor will abort if unwinding has already begun, for example. Also, I strongly suspect that Ferrocene-like projects do not rely on unwinding -- most production Rust projects I know do not, preferring panic=abort, or at least making very little use of unwinding for recovery. It'd be useful if you had an example of a particular piece of code where high availability was a requirement that also leveraged unwinding in a destructor in a crucial way.

@nikomatsakis
This is less about "what I rely on" and more about what I consider to be an acceptable mode of failure. I work on high reliability code for my current employer, and I worked on safety sensitive code for my previous employer. Unfortunately all of that work is proprietary so I'm not at liberty to provide examples from my own work.
I'm not suggesting that unwinding code would be an intended path, or a hot path for these contexts. What I am suggesting is that these contexts want to have more fine grained control over their failure mode. I apologize for not making that more clear.
You raise a good point here

Currently, panicking in a destructor will abort if unwinding has already begun, for example.

So I'll explain how I came to this view. While working on my codebase with high availability requirements I realized I had shot myself in the foot with https://github.com/rust-lang/rust/issues/86027. As I was contemplating my situation I realized that I would rather proceed in an unsound state than abort. That's probably a controversial choice, but it is important to me that I retain the ability to choose between my various evils. I don't consider any of my ways out of this scenario good, but I would prefer "might be bad" over "definitely bad". Wherein unsound behavior represents, to me at least, "might be bad". Aborting fits into "definitely bad".
I like the work happening in https://github.com/rust-lang/rust/issues/115285 because it gives me many options in how I approach the issue. My understanding is that the RFC we're discussing right now would turn all "panicked while panicking" scenarios into an immediate abort. So now instead of isolating panics to threads, I have to isolate them to processes.

Right, my apologies I had gotten a couple of proposals mixed up. However that strengthens my stance on this because the scenario actually being described in this RFC is more likely than I previously believed.

In sum the primary thrust of my argument has been weakened because there are a handful of scenarios relating to this where we already implicitly abort. So I suppose what I'm left with is "I would prefer to not add another scenario where we implicitly abort."
This is mostly because abort provides no recovery or logging path. I am deprived of my ability to make further decisions or automate recovery within the context.

This is mostly because abort provides no recovery or logging path.

The panic hook is still invoked during an abort, so logging would still be possible. Today it'd be 3 times for a panic-drop-panic nesting. With this RFC it'd be 2 times.
use std::panic;

struct Droppable;

impl Drop for Droppable {
    fn drop(&mut self) {
        eprintln!("in drop;");
        panic!("panic-in-drop");
    }
}

fn main() {
    panic::set_hook(Box::new(|payload| {
        eprintln!("Custom panic hook: {:?}", payload);
    }));

    let _foo = Droppable;

    panic!("Normal panic");
}

Custom panic hook: PanicInfo { payload: Any { .. }, message: Some(Normal panic), location: Location { file: "src/main.rs", line: 19, col: 5 }, can_unwind: true, force_no_backtrace: false }
in drop;
Custom panic hook: PanicInfo { payload: Any { .. }, message: Some(panic-in-drop), location: Location { file: "src/main.rs", line: 8, col: 9 }, can_unwind: true, force_no_backtrace: false }
Custom panic hook: PanicInfo { payload: Any { .. }, message: Some(panic in a destructor during cleanup), location: Location { file: "library/core/src/panicking.rs", line: 163, col: 5 }, can_unwind: false, force_no_backtrace: true }
thread caused non-unwinding panic. aborting.

Alright, yeah that becomes apparent upon re-reading the docs for std::panic::set_hook. Thank you for teaching me, I apologize that my learning had to happen at an inconvenient venue.

I actually find the current behavior of #![feature(c_unwind)] unwinding in extern "C" somewhat strange. Specifically, any unwinding edges within the extern "C" fn get sent to core::panicking::panic_cannot_unwind, meaning that the unwind happens up to the extern "C" fn, but any locals in said function don't get dropped. I would personally not expect wrapping the body of an extern "C" function in an inner extern "Rust" function to change behavior, but it does. (Concrete change: instead of terminating on all unwind edges individually, keep any unwind: bbN as-is and only turn unwind continue into unwind terminate(abi).)
The reason to mention how extern "C" turns unwinds into aborts is that I fully expect that the same would be the case for Drop::drop. The interesting decision, then, is which function is made nounwind: <_ as Drop>::drop or drop_in_place? My intuition would be that drop_in_place (and the MIR drop operation) would be the ones to turn an escaping unwind into a terminate.
Perhaps interestingly, the unwind-from-drop abort could entirely subsume the panic-during-unwind abort, at least when the underlying unwinding runtime supports well-nested unwinds (which Rust does support stably today on at least the tier-1 targets).

There is existing code that checks std::thread::panicking() to determine if it should cause a panic, usually for things like postcondition tests and/or debug assertions that might happen during drop cleanup. The former are mostly just out of luck for panic-in-drop=abort, but for the latter, it would be nice to have a std::thread::could_unwind() which returns false if an attempted unwind would always cause an abort. (Like with std::mem::needs_drop, it is allowed to always return true.) Even without this change it would be an improvement for nested catch_unwind scenarios.

If we want to change default Drop behavior, let's change it in 2024 edition

If we want to change default Drop behavior, let's change it in 2024 edition

See the Stabilize -Z panic-in-drop section. The same issue applies to editions.

I saw argument "Rust doesn't have linear types" many times in this and related discussions. But in fact, Rust do have (hack for) linear types! This is not officially supported in language, but a hack exist. Here is how you can create linear type A:
struct A;

impl Drop for A {
    fn drop(&mut self) {
        // Here goes random UUID to be sure this identifier is unique
        extern "C" {
            fn oops_drop_called_46ff17f6_6b60_4afb_853a_5c8b0f0eef18();
        }
        unsafe { oops_drop_called_46ff17f6_6b60_4afb_853a_5c8b0f0eef18() };
    }
}

fn main() {
    let x = A;
    std::mem::forget(x); // If we remove this "forget", we will get linker error
}

If A ever will be dropped, we will get linker error. This hack works in playground, both in debug and release modes. I didn't check it on other platforms.
Playground:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=d75f0c1da2a467c28dac170edaf3b617

That doesn't work when -Clink-dead-code=yes is used or if you are linking a rust dylib as in both cases the drop impl won't be omitted by the linker. In the former case rustc doesn't tell the linker to omit unused functions and in the later case the drop impl is exported from the dylib and thus preserved by the linker.

I actually find the current behavior of #![feature(c_unwind)] unwinding in extern "C" somewhat strange. Specifically, any unwinding edges within the extern "C" fn get sent to core::panicking::panic_cannot_unwind, meaning that the unwind happens up to the extern "C" fn, but any locals in said function don't get dropped. I would personally not expect wrapping the body of an extern "C" function in an inner extern "Rust" function to change behavior, but it does. (Concrete change: instead of terminating on all unwind edges individually, keep any unwind: bbN as-is and only turn unwind continue into unwind terminate(abi).)

We currently don't have any guarantee about destructor execution if unwinding will cause termination.
The current behaviour is that, on MSVC, triggering unwind in a non-unwindable context will immediate abort (this is the behaviour of SEH). On platforms using Itanium ABI with LLVM codegen, we currently execute destructors until the last frame, and we skip destructors in the last frame. With GCC codegen, we hoped to get the same immediate terminate behaviour as MSVC (this is not possible with LLVM since the LLVM IR doesn't have a way to encode "terminate" as an unwind action).
Changing the MIR codegen to call destructors for the last frame is a very simple change, but I don't think we want to make it a behaviour to be depended on.

The current behaviour is that, on MSVC, triggering unwind in a non-unwindable context will immediate abort

That doesn't quite match what I just got testing it (c67326b06 2024-03-15) on pc-windows-msvc; I do get a panic-during-unwind-cleanup abort after the second panic hook call (thus never seeing can_unwind: false nor the rtprintpanic! message), but an unwind-through-extern-c abort does still unwind to the terminate(abi) edge (thus drops locals lower on the stack than the extern "C" fn, calls the panic hook call, and outputs the rtprintpanic! message).
The way to unify that with your description is that panic-during-unwind is a non-unwindable context, but extern-Rust-in-extern-C is an unwindable context. If that's the case, then all panic unwinds which aren't ill-nested are raised in an unwindable context, as panic! involves calling Rust-ABI runtime functions from std/libpanic_unwind.
Thus my only observed difference between SEH and Itanium is whether the abort follows the source panic or a panic_in_cleanup nounwind panic.

Sorry for not making my message clear.
We currently implement extern "C"'s nounwind context essentially as try { ... } catch(...) { panic_cannot_unwind() } so yes they are technically unwindable. This is due to the LLVM limitation as mentioned earlier. If LLVM gains support then we probably will move over. GCC have eh_must_not_throw which will mark the callsite nounwind but AFAIK cg_gcc doesn't support it yet.
The cleanup context (w/ MSVC) is the only truly non-unwindable context you can get in Rust today I think.

@rfcbot fcp merge
This discussion has been ongoing for a long time and, personally, I believe it's time to move forward. I'm not sure the best tactical steps for rolling this out -- for example, maybe we want to begin with a lint for some time. But I think strategically we should commit to this direction.
To me, the absolute strongest reason to merge this is that it helps to ensure Rust's core value of reliability. Unwinding is very tricky to reason about, and unwinding from drops -- which are not visible in the source code -- is trickier still. @m-ou-se was making the case to me just recently that there are many, many bugs from this, and it is exceedingly difficult to get right. I know from personal experience that I tend to just create a "abort bomb" in cases where unsafe code owns values with unknown drop. I am sure I have mixed some cases. I am also quite confident that the subtleties of the current rule ("panic is allowed in drop but only if the drop is known not to execute during unwinding") are not well understood (we've seen it on this thread).
Another reason is that the vast majority of production users I have spoken with either (a) run with panic=abort, in which case this does not affect them; or (b) eventually abort the process anyway, in which case this likely does not affect them and in any case is not a major change; or (c) have bugs. ;) OK, (c) is editorializing.
I definitely see some cost here. It's a backwards incompatible change. There are times where panic in a drop is undeniably handy. I think we should pursue those independently as they arise (for example, there's been some recent discussion around adding a defer-like construct, which raises interesting questions about whether it should be able to panic as well).

To me, the absolute strongest reason to merge this is that it helps to ensure Rust's core value of reliability. Unwinding is very tricky to reason about, and unwinding from drops -- which are not visible in the source code -- is trickier still. @m-ou-se was making the case to me just recently that there are many, many bugs from this, and it is exceedingly difficult to get right. I know from personal experience that I tend to just create a "abort bomb" in cases where unsafe code owns values with unknown drop. I am sure I have mixed some cases. I am also quite confident that the subtleties of the current rule ("panic is allowed in drop but only if the drop is known not to execute during unwinding") are not well understood (we've seen it on this thread).

Out of curiosity, do you mean that there are 'many, many' actual bugs from unsafe code as it is instantiated in running programs, or do you mean that there are 'many, many' potential bugs if the unsafe code were called in a certain way? At least in my experience, it takes a very contrived setup for most panic-on-drop bugs to cause anything more harmful than a memory or resource leak, and obviously much code isn't leak-free in general on panic.
Meanwhile, the far bigger culprit I have seen for actual bugs has been unwinding outside Drop. Authors of unsafe code can very easily call eprintln!() or Vec::push(), not knowing that it might unwind and skip some cleanup code. Hence my earlier emphasis on adding better tools in general for unwinding: there will always be bugs as long as unwinding is invisible and mostly unstoppable, but I don't see removing the concept altogether as being the best solution. One crude but effective solution for the unwind problem might be something like an #[abort_on_unwind] attribute, that you can slap onto any function (or perhaps even any block?) to make it instantly abort if it is unwound into. (That is, make it abort as soon as possible, before dropping any other values.) Then, encourage anyone who doesn't want to worry about unwinding to put it on any function that temporarily relaxes an invariant.

I'm fine with this proposal as long as there existed a way to make a rust test fail reliably without any risk of causing abort. I don't think it's enough to default to panic-in-drop=unwind for tests, I wish there was a fail-safe test-only API to tell test frameworks that the currently executed test must eventually fail for the following reason, without touching the panicking infrastructure at all. There are many testing utilities relying on panic-in-drop for post-condition verification, because panicking is currently the only (reliable) way to fail a test.

If you're going forward with this change, please aid it by adding features for dealing with panics and drops:

something to annotate and enforce that certain code blocks or functions don't have any panicking code paths (at least a best effort lint if dyn or opaque types make it too hard).

an approximation of linear or no-drop types. Variations of this have been proposed many times. While it can't be bulletproof in Rust, even a basic implementation would be better than the current pattern of a having a runtime check with a panic in Drop.

many, many bugs from this, and it is exceedingly difficult to get right

Compare the case with the isize::MAX object size limit; there we eventually acquiesced that expecting all code to handle it correctly was too much and moved the requirement forward into Layout. That's a similar case of tightening the rules to make them more practical to follow, although the details of course differ.
The case of "even std got this wrong" would be the case of the caught unwind's drop glue panicking (previously) causing an unwind to escape start.

eventually abort the process anyway, in which case this likely does not affect them

Note that making drop glue (without an internal catch_unwind into a "non-unwinding context" reduces the number of drops done before the abort, even if the abort was going to happen either way. Because we (apparently) don't guarantee what drop glue does/n't get run when the unwind eventually causes an abort (see #), this could even result in no drop glue getting run.
I've personally settled into accepting turning an unwind through drop glue into an abort (e.g. by adding an abort call into the unwinding pads) as a practical choice. But not if an abort occurs without ever saying why, or at least calling the unwind handler with can_unwind=false.

The case of "even std got this wrong" would be the case of the caught unwind's drop glue panicking (previously) causing an unwind to escape start.

That wasn't the only drop-related unwind safety issue in std, was it? The RFC also mentions https://github.com/rust-lang/rust/issues/83618.

I personally really like the idea of implicit drops ("drop glue") not being allowed to unwind but explicit drops (drop/drop_in_place/ManuallyDrop::drop) being allowed.
I don't know how feasible this is, and I'm sure there are some further questions to solve (like "which functions constitute explicit drop points"?), but I do think it makes for a sensible and straightforward mental model, and makes it easy to choose the correct thing when e.g. writing unsafe or otherwise drop-sensitive code.

mem::drop doesn't have an explicit invocation of drop_in_place. It is defined as fn drop<T>(_arg: T) {} which implicitly drops the argument.

mem::drop doesn't have an explicit invocation of drop_in_place. It is defined as fn drop<T>(_arg: T) {} which implicitly drops the argument.

It was mentioned earlier in the thread that mem::drop could be rewritten in terms of an explicit drop, like ManuallyDrop::drop or drop_in_place. I was going off that.

I personally really like the idea of implicit drops ("drop glue") not being allowed to unwind but explicit drops (drop/drop_in_place/ManuallyDrop::drop) being allowed.

I think this is an idea worth pursuing.
Implementation-wise it should just be as simple as not making drop glues nounwind, but we emit drop terminators with UnwindAction::Terminate (like what we currently do for things in cleanup landing pads) if panic-in-drop=abort.
We then only need to make sure core::mem::drop calls drop_in_place instead of using implicit drop, or use some other mechanisms to ensure that it generates UnwindAction::Resume.

I personally really like the idea of implicit drops ("drop glue") not being allowed to unwind but explicit drops (drop/drop_in_place/ManuallyDrop::drop) being allowed.
I don't know how feasible this is, and I'm sure there are some further questions to solve (like "which functions constitute explicit drop points"?), but I do think it makes for a sensible and straightforward mental model, and makes it easy to choose the correct thing when e.g. writing unsafe or otherwise drop-sensitive code.

While I think this isn't a bad idea at its core, I'm wary of such a rule being applied everywhere, since then every time you see an explicit drop mechanism, you'd have to judge whether the author was specifically thinking about unwinding, or whether the explicit drop is just being used for readability or performance. (E.g., dropping a big Vec the moment you no longer need it.) Instead, as with my own suggestions, I'd prefer that this be something that can be opted into for particular spans of code, so that readers know immediately that it's using very drop-sensitive behavior.

This is the kind of thing where, similar to unsafe blocks, an explicit drop should almost always have an accompanying comment stating intent. Though I would also say that for example, safe code almost never actually cares about the panic behaviour of drop (or panic behaviour in general), so it's also fairly easy to tell from context

I personally really like the idea of implicit drops ("drop glue") not being allowed to unwind but explicit drops (drop/drop_in_place/ManuallyDrop::drop) being allowed.

I don't like this exceptional behavior, giving one main reason to use explicit drop/ManuallyDrop::drop is to control the drop order where the compiler cannot do it in the expected way otherwise. Eg. Early drop of Mutex where enclosing a block is not possible (loop related, or etc), pin the drop order of unsafe struct fields to be resistant from refactoring. Many of this use cases are not necessarily related to the panic handling, or on the contrary, they are expected/preferred to be panic-free.

(xref to some discussion of drop bombs in Tor Project gitlab: https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/2091#note_3021454)

Put another way, I really think we should be striving for a single set of rules that apply to Drop impls, no matter how they wind up being invoked. If we want differing sets of rules, perhaps we want another trait. So can we compare the value from distinguishing explicit drop vs having a finish method vs having a another trait (say Finish)?

The problem I see with that is that every generic container would need to forward Finish, since otherwise it wouldn't know to finish() all its elements. And extra opt-in traits tend not to get off the ground; see UnwindSafe for one failure mode. The main value in Drop is that it automatically forwards to all fields for safe containers, so that only unsafe containers need to worry about manual forwarding. Unless you propose that Finish should also be automatically implemented, but then generic containers would now need both a drop() and a finish() implementation.
Also, I'm still quite curious, what are those "many bugs" from unwind-on-drop that you were referring to?

A primary difficulty of using a fn finish(self) is that unless you play tricks with ManuallyDrop, the Drop code is still run after the finalization work done in the function and it's impossible to take any fields by value in order to .finish() them. We also lack any kind of defer or try-with-resources to help mitigate the need to remember to call into finalization. How often do you remember to .flush() an IO handle before dropping it?
I do think a native defer construct that was allowed to panic when run at end of scope would satisfy the use cases for deliberately panicking out of drop glue, leaving only the case of resiliency, which is already somewhat hobbled by double panics causing an (eventual) abort.
A notable failure case of opt-in no-drop-unwinds is that it's an uncontrolled property whether a type gets dropped in an unwind-friendly context or an unwind-abort context (and not one that can be tested for like thread::panicking), but to add to that, most type instances get stored in some sort of collection which is going to be unwind-during-cleanup sensitive, leading to that panics during cleanup still cause an abort more times than not, just maybe a little later.

A primary difficulty of using a fn finish(self) is that unless you play tricks with ManuallyDrop, the Drop code is still run after the finalization work done in the function and it's impossible to take any fields by value in order to .finish() them.

This is a gap in the language that could be filled in. As you observe, it's already possible but unsafe to do. It could be made safe, by providing a modifier on a struct or enum pattern (possibly restricted to use in the defining crate?) which:

bypasses the E0509 restriction, and
must move out all fields, except those whose types are Copy and thus have no destructors of their own, so that the field values are unambiguously now owned by the function rather than the matched value.

Just like a struct literal expression introduces a droppable struct value, by taking ownership of its ingredients as a primitive operation, this “de-construction” operation does the opposite.
Even if no changes are made to Rust's general finalization story, de-construction would be useful for all operations which consume a value and want to do something other than its Drop behavior — which currently has to be handled with either unsafe code or making fields Optional when they semantically aren't.
(To be clear, I'm not arguing in favor of adding finish() — I just had this idea sitting around for improving consuming functions in general. …I should write this up as an actual separate proposal, shouldn't I.)

well, since Drop is super weird and not really a normal trait, rust could just allow you to write:
impl Drop for MyType {
    fn drop(self) { // takes self by value, requires MyType: Unpin
        let Self { field_1, field_2 } = self;
        // ...
    }
}

Would it help to have some standard library type that panics in its own Drop (so all drop impls could catch panics and abort, except this one), but in a way that can be controlled externally?
struct Transaction {
    drop_bomb: Option<std::panic::Bomb>,
}

impl Transaction {
  fn commit(self) {
      self.drop_bomb.take().map(std::panic::Bomb::defuse);
  }
}

and/or have:
std::panic::no_unwind_on_drop(|| {
    // write delicate code here without worrying about unwinding
});

bypasses the E0509 restriction

My RFC #3466 would provide for this (though it would not address the rest of your post; the user would need to remember to perform the move).

A notable failure case of opt-in no-drop-unwinds is that it's an uncontrolled property whether a type gets dropped in an unwind-friendly context or an unwind-abort context (and not one that can be tested for like thread::panicking), but to add to that, most type instances get stored in some sort of collection which is going to be unwind-during-cleanup sensitive, leading to that panics during cleanup still cause an abort more times than not, just maybe a little later.

First off, as some others have said regarding this, not knowing if you're in an unwind-friendly context is already the status quo: you might have been compiled with -C panic=abort, so there's no guarantee there. Hence why adding a flag for aborting specifically on panic-on-drop is not being considered immediately breaking. And mandating abort-on-panicking-drop would introduce just as many unwind-abort contexts that cannot be tested for. (Or rather, you can test for it by checking the unstable PanicInfo::can_unwind(), but at that point it's far too late to do anything about it.) Also, it would be mostly equivalent to an aborting drop bomb (except possibly more eager, depending on the design choice), which is already a widespread approach for unwind-sensitive code that doesn't want to (or physically can't) carefully forward panics.
Second, I don't see unsafe-collection destructors as the kind of "unwind-during-cleanup sensitive" code that would want or need an abort bomb. Most collections don't have a crucial invariant to be maintained during drop: if a panic cuts them off, they just leak some of their elements and/or allocations. After all, the collection is totally inaccessible anyway once it's dropped, so any broken invariant would have to lie somewhere outside, which is far from common. I envision such an opt-in mechanism as specifically being for those cases where unsoundness or extreme incorrectness might occur from an unwind.
I suppose it could indeed be a failure mode if authors become afraid and start using it unnecessarily. Still, I think that the benefits of first-class explicit unwind control are worth this cost, especially given how unwieldy the current alternative (drop guards) is; we'd just have to take care in how we document it.

Second, I don't see unsafe-collection destructors as the kind of "unwind-during-cleanup sensitive" code that would want or need an abort bomb.

On the other hand one of the motivators for aborting upon unwind out of drop is preventing this kind of partial destruction and simplifying drop glue by not ever needing to handle unwinds during normal cleanup.

Also, I'm still quite curious, what are those "many bugs" from unwind-on-drop that you were referring to?

A subtle one I wrote somewhat recently (in proof of concept code) was assuming that dropping the sending half of a channel can't drop any queued messages; that dropping the receiving half drops the queue and sending fails after that sync point. For the usual impls, this is true unless dropping a message panics, in which case any remaining messages in the queue resume being dropped when the final sender is dropped.
Compare dropping VecDeque which does essentially let (front, back) = self.as_mut_slices(); defer { drop_in_place(back); } drop_in_place(front); to be fully unwind correct and doesn't just consecutively drop the two regions. Writing unwind correct drop implementations is difficult, even though safely forgetting destructors is generally simple enough (as long as you aren't projecting pinning invariants).

On the other hand one of the motivators for aborting upon unwind out of drop is preventing this kind of partial destruction and simplifying drop glue by not ever needing to handle unwinds during normal cleanup.

Sure, I'm all for making abort-on-unwinding-drop something that crates can opt into, given the measured perf benefits; it's just making it mandatory that I have big issues with. Regarding partial destruction, leaks might be particularly impactful in certain patterns or use cases, but it seems like an unlikely scenario where elements of the same collection might panic on drop, but also must never be leaked even during unwinding. That is, I don't see the status quo of 'best-effort' cleanup as particularly problematic: it just means that if you do have anything really important to clean up on panic, you might want to segregate it into its own collection.

A subtle one I wrote somewhat recently (in proof of concept code) was assuming that dropping the sending half of a channel can't drop any queued messages; that dropping the receiving half drops the queue and sending fails after that sync point. For the usual impls, this is true unless dropping a message panics, in which case any remaining messages in the queue resume being dropped when the final sender is dropped.

Thank you, that is a good example. I suppose channels would be an archetypical case of dropping a container possibly causing external effects.
More generally, in the status quo, when implementing a data structure with shared ownership (e.g., Arc), you have to make sure to make a contained value inaccessible before dropping it, to avoid any use-after-drop issues. But I still think that an opt-in abort at the call site would be a sufficient solution for these kinds of data structures.

Compare dropping VecDeque which does essentially let (front, back) = self.as_mut_slices(); defer { drop_in_place(back); } drop_in_place(front); to be fully unwind correct and doesn't just consecutively drop the two regions. Writing unwind correct drop implementations is difficult, even though safely forgetting destructors is generally simple enough

Yeah, writing fully unwind-correct code is pretty tedious currently. But I don't think it necessarily has to remain that way forever. There will always be lots of tricky unwind-sensitive code outside of destructors, so the more we can add built-in functionality to help with those, the less tedious drop implementations can become.
In the case of VecDeque, something akin to a defer or finally block would do the trick. But obviously, that wouldn't scale very cleanly to more than two elements, much less an entire list. I'm not really sure in general how easily we could tell LLVM, "For cleanup, just do the rest of the function (or rest of the loop, etc.) normally, then continue unwinding afterward." Though the idea does make me think of something silly like an Iterator::for_each_guarded() function, that just does the equivalent of
fn for_each_guarded(mut self) {
    struct Guard(Self);
    impl Drop for Guard { fn drop(&mut self) {
        while let Some(_item) = self.0.next() {}
    } }
    let this = Guard(self);
    while let Some(_item) = guard.0.next() {}
}

so you could then write
let (front, back) = self.as_mut_slices();
[front, back].into_iter().for_each_guarded(|slice| drop_in_place(slice));

(I'm also not sure how well the whole idea would optimize.)

(as long as you aren't projecting pinning invariants).

I suppose you're referring to certain forms of stack pinning in particular, where the data structure holds an owned value in borrowed storage? For heap pinning, the invariant would be upheld by just leaking the underlying memory after the destructor panics.

If a buggy destructor panics, I want to be able to isolate it without bringing down my entire application.

I generally agree with the "it's a breaking change argument," but there's an important caveat to the above statement: even today, recovering from a buggy destructor is not guaranteed, because double-panicking (due to a second buggy destructor) leads to an immediate abort.

(NOT A CONTRIBUTION) @Jules-Bertholet yes that's true in an absolute sense, but its about the degree of risk.
In a correctly implemented program, zero panics occur. But we need to tolerate a the risk of programmer error. Using unwinding to isolate the programmer error is an effect way to reduce the degree of risk in practice so that it is less likely to escalate into an urgent system failure.
With the current system, the risk that unwinding fails and becomes a process abort (increasing the chance of escalation) is the multiple of the probability of a panic, the probability of panic in a destructor, and the inverse of their correlation (because these two probabilities are likely somewhat correlated). By eliminating one of the multiples, this increases the risk of process abortion and therefore the risk of escalating failure.
Instead of reducing the degree to which unwinding is available to mitigate programmer error, the project should find ways to make correct unsafe code easier to write that doesn't require eliminating this mitigation.

... this is a breaking change to your existing user base and a violation of the Rust project's stability guarantees. For this reason alone this RFC should be closed & I don't understand how this can be brushed off so lightly.

Thank you for your intervention.  I agree.

drops (being silently inserted code) can cause bugs in unsafe code that hasn't considered them even if they don't panic.

Quite so,.

A totally non-invasive change would be to allow users to warn when any destructors are inserted in a code block, so they instead use explicit calls to drop.

This is of course an excellent idea.
This kind of alternative is IMO not at all properly discussed in the RFC.  There is one short sentence about it, nearly at the bottom of the list of alternatives.
The Alternatives section ought to explain, for each rejected alternative, why that option has been rejected.
In this case, the RFC does not give any reason why a lint wouldn't be a superior answer.  This is an obvious red flag for any RFC, let alone one which is such a significant departure from previously advertised guarantees, and which has attracted such substantial opposition.
The proper response to criticisms here in the discussion thread, is to refute them in the RFC text (if they can be refuted, which I suspect isn't the case here).  Instead, , the discussion here has become an unmanageable behemoth, and many good points made go unanswered.
TBH I have found it quite alarming, how ready the project seems to be to adopt a proposal which is so controversial, on the basis of a document which has such an obvious deficiency.  The RFC process is failing here.  I don't think this is because it's a bad process; rather, we don't seem to be applying it rigourously.

The Alternatives section ought to explain, for each rejected alternative, why that option has been rejected.

FWIW, I agree. I don't have a strong opinion either way for this RFC: from an unsafe code perspective, it'd be very nice to have to worry about fewer unwinding points -- but it's not clear that by "just" removing drops from the picture, we are making a significant enough dent here; there's still an unwinding point at each function call after all. But there are some interesting alternatives -- not just a lint; thanks to all the C-unwind work (that happened after this PR was initiated), the #[rustc_nounwind] attribute is now a safe attribute that turns all unwinding out of a function into aborts. So another possibility would be to let stable code not worry about unwinding by stabilizing some form of this attribute, possibly even on a scope smaller than an entire function (that should be fairly easy to implement with the same approach that backs rustc_nounwind).
As a t-lang advisor, I'd like to register this as a formal concern: to better describe the possible alternatives and explain why they are worse than the proposed solution.
@rfcbot concern alternatives
(AFAIK rfcbot doesn't understand the t-lang advisor role, so could a t-lang team member please tell the bot about this?)

(NOT A CONTRIBUTION)
I like Ralf's idea as a way of formalizing what libraries like rayon apparently already do: upgrading any panic to an abort whenever you enter their routines. But I think it's important that libraries document if they do this; given the choice between a library which handles unwinding correctly and one which doesn't, I would be more likely to choose the unwinding-friendly library, and I don't think we should abandon that as the goal for unsafe code.

@rfcbot concern describe alternatives
Ralf writeS:

As a t-lang advisor, I'd like to register this as a formal concern: to better describe the possible alternatives and explain why they are worse than the proposed solution.
(AFAIK rfcbot doesn't understand the t-lang advisor role, so could a t-lang team member please tell the bot about this?)

As an interesting note, C++ denied std::scope_guard because of the C++ STL rule that all destructors must be noexcept. I've firmly settled into a position that I don't mind drop glue being nounwind, but I also hold that we shouldn't make that change until there is some way to run code during scope cleanup that is still allowed to unwind (e.g. a defer).

Something Nadrieril came up with during unsafe fields rfc discussion:
How about unsafe locals (unsafe let) that cause an abort if they're still alive at the end of a function (be it by return or unwind). Which means one has to explicitly drop or move them.

(to be clear, my intent was for the borrow-checker to prevent automatic drops out of unsafe let locals by:

forcing us to drop or move out of each unsafe let local explicitly;
turning all unwinds into aborts while the locals are live.)


devtools does not show fonts using css "font:", but it does with css "font-family"

I created a minimal nuxt setup. Installed nuxt/fonts module.
Fonts referenced by css font-family appear in the fonts devtools panel.
Fonts referenced by css font do not appear.
This is not a new issue.  I have noticed this behaviour before.  I have just recently had time to created a minimal example and work out what (it seems to me) is not working.
npm list:
<EMAIL_ADDRESS><EMAIL_ADDRESS><EMAIL_ADDRESS><EMAIL_ADDRESS>app.vue:
<template>

    <NuxtRouteAnnouncer />

This is a test

</template>

It seems to me that toggling betwen "font:" and "font-family:" in the css makes Lato (or any other font) appear or disappear in the devtools fonts tab.
  Or that the fonts module does not respect "fonts:" by design.
Sometimes "npm run dev" is required to show this behaviour.
Examples:

Just to add, the issue is still there in @nuxt/fonts 0.9.2
With thanks for your help. . .

You're right, nuxt/fonts does not scan the font: declaration. It should.


SW-522 - Pull latest version of moveit

Description
Please explain the changes you made, including a reference to the related issue if applicable
Checklist

[ ] Required: Code is auto formatted using clang-format
[ ] Extended the tutorials / documentation, if necessary reference
[ ] Include a screenshot if changing a GUI
[ ] Optional: Created tests, which fail without this PR reference
[ ] Optional: Decide if this should be cherry-picked to other current ROS branches (Indigo, Jade, Kinetic)

Should go in at the same time as https://github.com/iron-ox/iron_ox/pull/921

@bhomberg This compiles fine for me, and i'm able to run reachability anlysis, but experience the same rviz problem you did: https://ironox.atlassian.net/browse/SW-580  I'm digging into that now; I'd like to at least understand the cause before we merge.

I don't think the moveit change caused the rviz issues -- I was seeing them just after running a rosdep update last week.  However, this PR will cause people to do a rosdep update, so I agree that we should figure the rviz thing out first.


Remove subdependency pins

Subdependencies of a package shouldn't be pinned as it increases the likelihood of an unresolvable dependency set.

 Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.


Install Thorium in Windows 10

Hi @Alex313031 , Is there an installation guide or Wiki for Thorium? because i little confused about installation.
i just downloaded from Thorium-Win/releases and from Thorium-AVX2/releases but when i open the file, it doesn't show anything.
and i want to ask what's the difference of zip file and mini installer exe file in Thorium-AVX2/releases ?
btw, in Thorium-Win/releases, there are 2 files namely .zip.exe and .exe.zip make me confused too

@vjrmy The .exe is an installer, just double click and it will install Thorium silently and then open a new Thorium window. The .zip is a portable .zip so that you can run it on your system without installing. There is a README inside describing usage.
Therefore, there are two files, one with .exe and one with .zip extension. If your browser is downloading them as .exe.zip and .zip.exe, then I would say its a problem with your current browser and to try downloading them in a different browser.

it turns out that my laptop processor does not support avx or avx2 that's why thorium is not installed. my processor is intel celeron n3350
I will be happy if you release a version for my old processor. Thank you

@vjrmy A couple of people have asked for this too. I'll make an SSE4 version for yall and post it in thorium-special.

I'll be waiting for that

@vjrmy

I found it. thanks my friend for telling me

but the package is debian .deb which can't be installed on windows 10. Is there a package that is .exe?

but the package is debian .deb which can't be installed on windows 10. Is there a package that in .exe?

@vjrmy Thorium has installation packages in deb and exe format, as well as portable version for windows. The installation package can be downloaded from the following page.
AVX version https://github.com/Alex313031/Thorium/releases/tag/M103.0.5036.0
AVX2 version
https://github.com/Alex313031/Thorium-AVX2/releases/tag/M103.0.5045.0

@vjrmy OMFG I'm dumb. I just realized this issue was in Thorium-AVX2 so obviously you are looking for a SSE4 windows release. I will make one soon for you.

@vjrmy Hey, @gz83 Is making an sse4 only release as well as an ARM release for windows.

@vjrmy Ok here you go > https://github.com/Alex313031/Thorium-Special/releases/tag/M104.0.5091.0


Add enums for webhook payloads action field

Relevant Issue: https://github.com/google/go-github/issues/3127

@gmlewis @willnorris any update on the PR?

this can be done progressively as well if you prefer and do it one event at a time (or in batches)


RandomCoin as a trait

This issue aims to start a discussion on the merits of changing RandomCoin<B: StarkField, H: Hasher> from a struct to a trait. The reasons for this being that for some choices of H (specifically the ones based on the sponge construction) as well as of B (e.g. when it is the base field of the algebraic hasher H) seeding (which can be done with absorbing) and drawing random values (which can be done by squeezing) can be achieved in more efficient ways.

This would be great! I've been thinking about something like this as well. Maybe the trait could look like this:
pub trait RandomCoin {
    type BaseField: StarkField;
    type Hasher: Hasher;

    // REQUIRED METHODS
    // --------------------------------------------------------------------------------------------

    fn new(seed: &[Self::BaseField]) -> Self;

    fn reseed(&mut self, data: <Self::Hasher as Hasher>::Digest);

    fn reseed_with_int(&mut self, value: u64);

    fn check_leading_zeros(&self, value: u64) -> u32;

    fn leading_zeros(&self) -> u32;

    fn draw<E: FieldElement<BaseField = Self::BaseField>>(&mut self) -> Result<E, RandomCoinError>;

    // PROVIDED METHODS
    // --------------------------------------------------------------------------------------------

    fn draw_pair<E>(&mut self) -> Result<(E, E), RandomCoinError>
    where
        E: FieldElement<BaseField = Self::BaseField>,
    {
        Ok((self.draw()?, self.draw()?))
    }

    fn draw_triple<E>(&mut self) -> Result<(E, E, E), RandomCoinError>
    where
        E: FieldElement<BaseField = Self::BaseField>,
    {
        Ok((self.draw()?, self.draw()?, self.draw()?))
    }

    fn draw_integers(
        &mut self,
        num_values: usize,
        domain_size: usize,
    ) -> Result<Vec<usize>, RandomCoinError> {
        todo!()
    }
}

Closed by #176.


util/commit-validator: Fix DCO link

At some point the sanitizer for anchor generation changed, this adds a missing apostrophe to the anchor.

LGTM


IPC - SteamID of bot = 0

Bug
Description
Some times SteamID field of !api response of ASF IPC is 0
Expected behavior
Corrent SteamID
Actual behavior
https://i.imgur.com/d6liSst.png
Additional info
I cant deduce the pattern why that happens. SteamID can becomes 0 even after week of correct work. Restart of farm doesnt solve the problem at all. Time by time it helps. But some times it doent.

You didn't provide full log as stated in contributing section, neither reproduction pattern, so I'm marking it as not a bug until you do. https://github.com/JustArchi/ArchiSteamFarm/wiki/API


fix 1px bump on select2

WHY
BEFORE - What was wrong? What was happening before this PR?
Reported in https://github.com/Laravel-Backpack/CRUD/issues/4710

Found and fixed 🎉 Though... now that I think about it... it's a huge red flag that we have the purple color here... we should probably use CSS variables so that the border is PURPLE on the purple color scheme, blue on the others... right?

Done! Pedro, please double-check it works of for you too?


chore(deps): upgrade dependencies

Upgrades project dependencies. See details in workflow run.

Automatically created by projen via the "upgrade-k8s-24-main" workflow

⚪ Backport skipped
The pull request was not backported as there were no branches to backport to. If this is a mistake, please apply the desired version labels or run the backport tool manually.
Manual backport
To create the backport manually run:
backport --pr 1430

Questions ?
Please refer to the Backport tool documentation


Installation instructions for text-icu-0.8.0 ?

What is necessary to install v0.8.0?
$ cabal install text-icu-0.8.0
Resolving dependencies...
Error: cabal: Could not resolve dependencies:
[__0] next goal: text-icu (user goal)
[__0] rejecting: text-icu-0.8.0 (conflict: pkg-config package icu-i18n-any,
not found in the pkg-config database)

The change log says: https://github.com/haskell/text-icu/blob/7830071e9e13db07cf661fa9ec6aba3a0a5e36a1/changelog.md#L7

But the README does not have updated installation instructions.
The linked PR is mute on the why and how: https://github.com/haskell/text-icu/pull/43
Same for the original issue: #42

See also the build failure in the Agda CI: https://github.com/agda/agda/runs/5042362762?check_suite_focus=true#step:7:408

There's the same error in Hackage
https://hackage.haskell.org/package/text-icu-0.8.0/reports/1
It says:
NumberFormatter.hsc:25:10: fatal error: unicode/unumberformatter.h: No such file or directory

unicode/unumberformatter.h was added in ICU 60. So I suppose that an older ICU version is installed on both Hackage and Agda CI.
Could you check what libicu version is installed in Agda CI? Perhaps you need to update it.
It's also interesting whom I could ask to update libicu on the Hackage build server.

It's also interesting whom I could ask to update libicu on the Hackage build server.

Maybe @davean ?

@vshabanov wrote:

NumberFormatter.hsc:25:10: fatal error: unicode/unumberformatter.h: No such file or directory

unicode/unumberformatter.h was added in ICU 60. So I suppose that an older ICU version is installed on both Hackage and Agda CI.
Could you check what libicu version is installed in Agda CI? Perhaps you need to update it.

Indeed, this error was fixed by going from the ubunutu-2018 vm (Ubuntu 18.04 LTS) to ubuntu-latest (Ubuntu 20.04 LTS), see https://github.com/agda/agda/runs/5066391230?check_suite_focus=true .

I've made a little update of README in 6219dbfcfb577759991f6a0afdc50396ae961184, and opened an issue about updating libicu on the Hackage build server https://github.com/haskell/hackage-server/issues/999

See #65 for stack/windows.
I still have to figure out cabal/windows.


null check while validating the move , horde constructor fix

Same check exists in ChessGame but was missing in Atomic

Found another bug in Horde constructor, added to the same PR

Looks good, thanks!
About that if count > 0 check in HordeChessGame to be consistent with the ChessGame constructor, I'm not entirely sure why I did that in ChessGame because there's really no reason to. Guess I can go remove that.


docs: typo fixup in crictl.md

reading through the docs on setting up runtime handler and noticed the typo.
Cheers,
Eric

/assign @feiskyer

/approve

@egernst thanks for the fix


DOC: adding @nbokulich's document with some of my modifications

This is @nbokulich's document. I went through this in detail and made some small changes throughout.

@gregcaporaso done reviewing

@jairideout, addressed your comments. Thanks for the review!

Thanks Jai and Greg!
On Tue, May 17, 2016 at 5:12 PM, Greg Caporaso<EMAIL_ADDRESS>wrote:

@jairideout https://github.com/jairideout, addressed your comments.
Thanks for the review!
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/caporaso-lab/mockrobiota/pull/20#issuecomment-219890988

Looks great, thanks @gregcaporaso @nbokulich!


Enhance ability to use client to make authenticated requests

add Server::getHeaders() for making authenticated requests

See also thephpleague/oauth2-client#232

@bencorlett my PR for oauth2-client has been accepted by @ramsey. Would you be able to merge this too, so that oauth1-client has a similar interface?

SAWWEEETTT :)


Bump conn req to 4 3 0

This also fixes the .travis.yml file for this project to actually run the integration specs.

LGTM