Exclude build metadata from equality checking #12454
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
On the |
|
@alexcrichton actually the semver spec defines only If we use build metadata for equality checking, I confirmed that node-semver ignores build metadata, but it is not the reference implementation of course. |
|
That's reasonable-enough rationale for me, could you add some of that in comments? |
|
OK. |
bors
added a commit
that referenced
this pull request
Feb 24, 2014
|
The test failure wasn't your fault, but this appears to need a rebase now. |
Build metadata is already excluded from precedence checking in line with the spec. For consistency and providing strict total ordering for Version, build metadata should also be ignored in Eq impl. Closes #12438 Signed-off-by: OGINO Masanori <masanori.ogino@gmail.com>
|
Done. r? |
bors
added a commit
that referenced
this pull request
Feb 25, 2014
dingxiangfei2009
pushed a commit
to dingxiangfei2009/rust
that referenced
this pull request
Jul 28, 2024
This commit fixes a bug introduced in rust-lang#12706, where the behavior of the lint has been changed, to avoid suggestions that introduce a move. The motivation in the commit message is quite poor (if the detection for significant drops is not sufficient because it's not transitive, the proper fix would be to make it transitive). However, rust-lang#12454, the linked issue, provides a good reason for the change — if the value being borrowed is bound to a variable, then moving it will only introduce friction into future refactorings. Thus rust-lang#12706 changes the logic so that the lint triggers if the value being borrowed is Copy, or is the result of a function call, simplifying the logic to the point where analysing "is this the only use of this value" isn't necessary. However, said PR also introduces an undocumented carveout, where referents that themselves are mutable references are treated as Copy, to catch some cases that we do want to lint against. However, that is not sound — it's possible to consume a mutable reference by moving it. To avoid emitting false suggestions, this PR reintroduces the referent_used_exactly_once logic and runs that check for referents that are themselves mutable references. Thinking about the code shape of &mut x, where x: &mut T, raises the point that while removing the &mut outright won't work, the extra indirection is still undesirable, and perhaps instead we should suggest reborrowing: &mut *x. That, however, is left as possible future work. Fixes rust-lang#12856
dingxiangfei2009
pushed a commit
to dingxiangfei2009/rust
that referenced
this pull request
Jul 28, 2024
… r=xFrednet needless_borrows_for_generic_args: Fix for &mut This commit fixes a bug introduced in rust-lang#12706, where the behavior of the lint has been changed, to avoid suggestions that introduce a move. The motivation in the commit message is quite poor (if the detection for significant drops is not sufficient because it's not transitive, the proper fix would be to make it transitive). However, rust-lang#12454, the linked issue, provides a good reason for the change — if the value being borrowed is bound to a variable, then moving it will only introduce friction into future refactorings. Thus rust-lang#12706 changes the logic so that the lint triggers if the value being borrowed is Copy, or is the result of a function call, simplifying the logic to the point where analysing "is this the only use of this value" isn't necessary. However, said PR also introduces an undocumented carveout, where referents that themselves are mutable references are treated as Copy, to catch some cases that we do want to lint against. However, that is not sound — it's possible to consume a mutable reference by moving it. To avoid emitting false suggestions, this PR reintroduces the referent_used_exactly_once logic and runs that check for referents that are themselves mutable references. Thinking about the code shape of &mut x, where x: &mut T, raises the point that while removing the &mut outright won't work, the extra indirection is still undesirable, and perhaps instead we should suggest reborrowing: &mut *x. That, however, is left as possible future work. Fixes rust-lang#12856 changelog: none
Jarcho
pushed a commit
to Jarcho/rust
that referenced
this pull request
Aug 26, 2024
… r=xFrednet needless_borrows_for_generic_args: Fix for &mut This commit fixes a bug introduced in rust-lang#12706, where the behavior of the lint has been changed, to avoid suggestions that introduce a move. The motivation in the commit message is quite poor (if the detection for significant drops is not sufficient because it's not transitive, the proper fix would be to make it transitive). However, rust-lang#12454, the linked issue, provides a good reason for the change — if the value being borrowed is bound to a variable, then moving it will only introduce friction into future refactorings. Thus rust-lang#12706 changes the logic so that the lint triggers if the value being borrowed is Copy, or is the result of a function call, simplifying the logic to the point where analysing "is this the only use of this value" isn't necessary. However, said PR also introduces an undocumented carveout, where referents that themselves are mutable references are treated as Copy, to catch some cases that we do want to lint against. However, that is not sound — it's possible to consume a mutable reference by moving it. To avoid emitting false suggestions, this PR reintroduces the referent_used_exactly_once logic and runs that check for referents that are themselves mutable references. Thinking about the code shape of &mut x, where x: &mut T, raises the point that while removing the &mut outright won't work, the extra indirection is still undesirable, and perhaps instead we should suggest reborrowing: &mut *x. That, however, is left as possible future work. Fixes rust-lang#12856 changelog: none
Jarcho
pushed a commit
to Jarcho/rust
that referenced
this pull request
Aug 26, 2024
… r=xFrednet needless_borrows_for_generic_args: Fix for &mut This commit fixes a bug introduced in rust-lang#12706, where the behavior of the lint has been changed, to avoid suggestions that introduce a move. The motivation in the commit message is quite poor (if the detection for significant drops is not sufficient because it's not transitive, the proper fix would be to make it transitive). However, rust-lang#12454, the linked issue, provides a good reason for the change — if the value being borrowed is bound to a variable, then moving it will only introduce friction into future refactorings. Thus rust-lang#12706 changes the logic so that the lint triggers if the value being borrowed is Copy, or is the result of a function call, simplifying the logic to the point where analysing "is this the only use of this value" isn't necessary. However, said PR also introduces an undocumented carveout, where referents that themselves are mutable references are treated as Copy, to catch some cases that we do want to lint against. However, that is not sound — it's possible to consume a mutable reference by moving it. To avoid emitting false suggestions, this PR reintroduces the referent_used_exactly_once logic and runs that check for referents that are themselves mutable references. Thinking about the code shape of &mut x, where x: &mut T, raises the point that while removing the &mut outright won't work, the extra indirection is still undesirable, and perhaps instead we should suggest reborrowing: &mut *x. That, however, is left as possible future work. Fixes rust-lang#12856 changelog: none
MabezDev
pushed a commit
to esp-rs/rust
that referenced
this pull request
Sep 3, 2024
… r=xFrednet needless_borrows_for_generic_args: Fix for &mut This commit fixes a bug introduced in rust-lang#12706, where the behavior of the lint has been changed, to avoid suggestions that introduce a move. The motivation in the commit message is quite poor (if the detection for significant drops is not sufficient because it's not transitive, the proper fix would be to make it transitive). However, rust-lang#12454, the linked issue, provides a good reason for the change — if the value being borrowed is bound to a variable, then moving it will only introduce friction into future refactorings. Thus rust-lang#12706 changes the logic so that the lint triggers if the value being borrowed is Copy, or is the result of a function call, simplifying the logic to the point where analysing "is this the only use of this value" isn't necessary. However, said PR also introduces an undocumented carveout, where referents that themselves are mutable references are treated as Copy, to catch some cases that we do want to lint against. However, that is not sound — it's possible to consume a mutable reference by moving it. To avoid emitting false suggestions, this PR reintroduces the referent_used_exactly_once logic and runs that check for referents that are themselves mutable references. Thinking about the code shape of &mut x, where x: &mut T, raises the point that while removing the &mut outright won't work, the extra indirection is still undesirable, and perhaps instead we should suggest reborrowing: &mut *x. That, however, is left as possible future work. Fixes rust-lang#12856 changelog: none
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #12438