Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make unwinding task-independent #12080

Closed
brson opened this issue Feb 7, 2014 · 3 comments
Closed

Make unwinding task-independent #12080

brson opened this issue Feb 7, 2014 · 3 comments
Labels
A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows

Comments

@brson
Copy link
Contributor

brson commented Feb 7, 2014

For the sake of embedding applications it would be nice if Rust's unwinding implementation was not directly coupled to the task abstraction.

Right now unwinding is mostly just try/catch, but it also sets a flag on the task to indicate that the task is failing. It would be better if std could fulfill its unwinding obligations without involving the task.

@alexcrichton
Copy link
Member

There are a two questions I can think of that the unwinding code needs to have answers to:

  • Am I currently unwinding? -- abort on double failure
  • Am I currently in a try/catch block? -- abort on try/catch patterns

Perhaps these can be solved with thread-local globals? Right now we only have to worry about one thread local global which is quite convenient.

Additionally, exposing a safe interface around this is going to be tough. I think that we can get by with:

fn try(f: ||) -> bool;

I remember there being issues around stack closures leaving captured things in an unusual state, but this doesn't seem much different to me than normal failure, so I may be forgetting why stack closures on try boundaries is a bad idea.

@alexcrichton
Copy link
Member

Note that we now have a try function, but it's unsafe and sill relies on Task for safety (see the comments on that function).

@steveklabnik
Copy link
Member

I'm pulling a massive triage effort to get us ready for 1.0. As part of this, I'm moving stuff that's wishlist-like to the RFCs repo, as that's where major new things should get discussed/prioritized.

This issue has been moved to the RFCs repo: rust-lang/rfcs#632

flip1995 pushed a commit to flip1995/rust that referenced this issue Jan 11, 2024
Fixed ICE introduced in rust-lang#12004

Issue: in rust-lang/rust-clippy#12004, we emit a lint for `filter(Option::is_some)`. If the
parent expression is a `.map` we don't emit that lint as there exists a
more specialized lint for that.

The ICE introduced in rust-lang/rust-clippy#12004 is a consequence of the assumption that a
parent expression after a filter would be a method call with the filter
call being the receiver. However, it is entirely possible to have a
closure of the form

```
|| { vec![Some(1), None].into_iter().filter(Option::is_some) }
```
The previous implementation looked at the parent expression; namely the
closure, and tried to check the parameters by indexing [0] on an empty
list.

This commit is an overhaul of the lint with significantly more FP tests
and checks.

Impl details:

1. We verify that the filter method we are in is a proper trait method
   to avoid FPs.
2. We check that the parent expression is not a map by checking whether
   it exists; if is a trait method; and then a method call.
3. We check that we don't have comments in the span.
4. We verify that we are in an Iterator of Option and Result.
5. We check the contents of the filter.
   1. For closures we peel it. If it is not a single expression, we don't
     lint. We then try again by checking the peeled expression.
   2. For paths, we do a typecheck to avoid FPs for types that impl
     functions with the same names.
   3. For calls, we verify the type, via the path, and that the param of
     the closure is the single argument to the call.
   4. For method calls we verify that the receiver is the parameter of
     the closure. Since we handle single, non-block exprs, the
     parameter can't be shadowed, so no FP.

This commit also adds additional FP tests.

Fixes: rust-lang#12058

Adding `@xFrednet` as you've the most context for this as you reviewed it last time.

`@rustbot` r? `@xFrednet`

---

changelog: none
(Will be backported and therefore don't effect stable)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows
Projects
None yet
Development

No branches or pull requests

3 participants