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

Explain "It is a logic error ... " in HashMap, BTreeMap and others #80657

Closed
ChrisJefferson opened this issue Jan 3, 2021 · 2 comments
Closed
Labels
A-collections Area: `std::collection` A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@ChrisJefferson
Copy link
Contributor

In various pieces of the documentation that talk about "It is a logic error ...", mostly to change the keys of HashMap/Sets and BTreeMap/Sets, using RefCells or hashing functions which change their behaviour based on global variables.

What does "it is a logic error ... " mean? With some brief playing it appears (to me) that it puts the datastructure in an inconsistent state, where future calls to any member function can return nonsense values.

It appears (from what I can tell) there is no unsafe behaviour, and it isn't possible to make the datastructures call panic, but I would be happy to say they could panic (saying they won't means checking they can't when someone has already done something stupid).

So, I suggest a short extra sentence which just says "These logic errors can lead to incorrect results and panics but not unsafe behaviour"

@sfackler sfackler added the A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools label Jan 3, 2021
@camelid
Copy link
Member

camelid commented Jan 3, 2021

By "unsafe behavior" I assume you mean "Undefined Behavior"?

@camelid camelid added A-collections Area: `std::collection` C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jan 3, 2021
@Mark-Simulacrum
Copy link
Member

We must guarantee that there is no UB as a result of nonsense Ord/Hash/PartialEq etc impls, but I do not think we should guarantee anything beyond that. I would be open to a PR making a change improving the wording here.

m-ou-se added a commit to m-ou-se/rust that referenced this issue Jan 16, 2021
…-ou-se

Clarify what the effects of a 'logic error' are

This clarifies what a 'logic error' is (which is a term used to describe what happens if you put things in a hash table or btree and then use something like a refcell to break the internal ordering). This tries to be as vague as possible, as we don't really want to promise what happens, except "bad things, but not UB". This was discussed in rust-lang#80657
m-ou-se added a commit to m-ou-se/rust that referenced this issue Jan 16, 2021
…-ou-se

Clarify what the effects of a 'logic error' are

This clarifies what a 'logic error' is (which is a term used to describe what happens if you put things in a hash table or btree and then use something like a refcell to break the internal ordering). This tries to be as vague as possible, as we don't really want to promise what happens, except "bad things, but not UB". This was discussed in rust-lang#80657
wsh pushed a commit to wsh/rust-reference that referenced this issue Jan 17, 2021
In rust-lang/rust#80657 and
rust-lang/rust#80681 it is discussed
how to clarify/define what a "logic error" is and what are
their consequences. The reference should mention them as well.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-collections Area: `std::collection` A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants