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.

Sign up for GitHub

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

Jump to bottom

Define Ord for &mut T if T is Ord #14074

Closed
nikomatsakis opened this issue May 10, 2014 · 1 comment · Fixed by #14133
Closed

Define Ord for &mut T if T is Ord #14074

nikomatsakis opened this issue May 10, 2014 · 1 comment · Fixed by #14133
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@nikomatsakis
Copy link
Contributor

Currently, we don't define Ord for &mut T if T is Ord. But we should. As a curious side-effect, that permits you to write things like:

fn min<T:Ord>(x: T, y: T) -> T {
    if x < y { x } else { y }
}

pub fn main() {
    let mut x = 3;
    let mut y = 4;
    let mut z = 5;

    *min(&mut x, &mut y) = min(0, z);
}
@nikomatsakis
Copy link
Contributor Author

(More generally, any time we have an impl like impl<T:Foo> Foo for &T, we probably want one for &mut T. Certainly for the operators.)

@db48x db48x mentioned this issue May 12, 2014
Merged
alan-andrade added a commit to alan-andrade/rust that referenced this issue May 12, 2014
@alan-andrade
@wip for ticket rust-lang#14074
fd4dc46
db48x added a commit to db48x/rust that referenced this issue May 14, 2014
@db48x
define Eq,TotalEq,Ord,TotalOrd for &mut T
9eb723d 
Also Show, which is useful in assertions. Fixes rust-lang#14074
bors added a commit that referenced this issue May 15, 2014
@bors
auto merge of #14133 : db48x/rust/ord-for-mut-refs, r=alexcrichton
73a68cd 
Also Show, which is useful in assertions. Fixes #14074
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 20, 2023
@bors
Auto merge of rust-lang#14122 - lowr:patch/abort-macro-expansion-on-o…
2a57b01 
…verflow, r=Veykril

fix: Don't expand macros in the same expansion tree after overflow

This patch fixes 2 bugs:

- In `Expander::enter_expand_id()` (and in code paths it's called), we never check whether we've reached the recursion limit. Although it hasn't been reported as far as I'm aware, this may cause hangs or stack overflows if some malformed attribute macro is used on associated items.
- We keep expansion even when recursion limit is reached. Take the following for example:

  ```rust
  macro_rules! foo { () => {{ foo!(); foo!(); }} }
  fn main() { foo!(); }
  ```

  We keep expanding the first `foo!()` in each expansion and would reach the limit at some point, *after which* we would try expanding the second `foo!()` in each expansion until it hits the limit again. This will (by default) lead to ~2^128 expansions.

  This is essentially what's happening in rust-lang#14074. Unlike rustc, we don't just stop expanding macros when we fail as long as it produces some tokens so that we can provide completions and other services in incomplete macro calls.

This patch provides a method that takes care of recursion depths (`Expander::within_limit()`) and stops macro expansions in the whole macro expansion tree once it detects recursion depth overflow. To be honest, I'm not really satisfied with this fix because it can still be used in unintended ways to bypass overflow checks, and I'm still seeking ways such that misuses are caught by the compiler by leveraging types or something.

Fixes rust-lang#14074
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

define Eq,TotalEq,Ord,TotalOrd for &mut T db48x/rust
2 participants
@nikomatsakis @thestinger