Skip to content

Conversation

@kmcallister
Copy link
Contributor

I needed these to implement efficient poisoning in seqloq.

@rust-highfive
Copy link
Contributor

r? @huonw

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member

I think adding PoisonError::new is probably fine, but can this avoid exposing map_lock_result for now? That should be implement-able with new exposed, right?

cc @aturon, an interesting conventions dilemma of whether we should provide constructors for all errors or largely just some method to reconstruct "wrapper errors" after decomposing them.

@kmcallister
Copy link
Contributor Author

can this avoid exposing map_lock_result for now? That should be implement-able with new exposed, right?

Yeah, I could copy it out, but I don't want to ;)

@alexcrichton
Copy link
Member

As an API we'll have to support to the end of time the threshold should be fairly high for inclusion. A bare map_lock_result doesn't really fit into any conventions that we have so far, so it should likely be left out until a later date if it is necessary to re-include.

@kmcallister
Copy link
Contributor Author

As an API we'll have to support to the end of time

It's marked as #[unstable]. I thought we could add things like this as part of the "stability not stagnation".

@alexcrichton
Copy link
Member

It's marked as #[unstable]. I thought we could add things like this as part of the "stability not stagnation".

Although we have a distinction of #[unstable] APIs in the standard library it does not mean that it will become a "dumping ground" for everything submitted. The lifetime of a new API is:

  1. An RFC is created for a new API and is discussed.
  2. The RFC is accepted.
  3. The RFC is implemented with #[unstable] APIs
  4. Some time later, the APIs are removed or promoted to #[stable]

All currently #[unstable] APIs are either planned for removal or planned to be revisited at some point in the future to stabilize.

Along these lines, I do not think that this map_lock_result is an API that we would want to stabilize, so it should not be included at this time. The PoisonError::new API is a fine candidate for stabilization, however, as it follows a well accepted pattern of constructors and is a very minor (and probably expected) addition to the API.

@huonw
Copy link
Contributor

huonw commented Feb 1, 2015

r? @alexcrichton

@rust-highfive rust-highfive assigned alexcrichton and unassigned huonw Feb 1, 2015
@aturon
Copy link
Contributor

aturon commented Feb 2, 2015

(FWIW, I agree with @alexcrichton here.)

@kmcallister
Copy link
Contributor Author

That sounds good, I'll update this PR soon.

@kmcallister
Copy link
Contributor Author

Updated to keep map_lock_result private.

@alexcrichton
Copy link
Member

@bors: r+ 7324c2c

@bors
Copy link
Collaborator

bors commented Feb 8, 2015

⌛ Testing commit 7324c2c with merge d4f9ec5...

bors added a commit that referenced this pull request Feb 8, 2015
@bors
Copy link
Collaborator

bors commented Feb 8, 2015

@bors bors merged commit 7324c2c into rust-lang:master Feb 8, 2015
lnicola pushed a commit to lnicola/rust that referenced this pull request Feb 9, 2026
Remove outdated SyntaxErrorKind FIXME comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants