Skip to content
This repository has been archived by the owner on Aug 16, 2021. It is now read-only.

Use associated function syntax for Debug and Display backtrace impl #279

Conversation

azriel91
Copy link

@azriel91 azriel91 commented Dec 11, 2018

Rust 1.31 reports a compilation error saying it is ambiguous which
fmt method to call.


Not sure if this happened prior to Rust 1.31, but when compiling failure using Rust 1.31.0 (stable) this error shows up:

[07:33:30] failure master
$ cargo +stable test --all
...
error[E0034]: multiple applicable items in scope
   --> src\backtrace\mod.rs:132:20
    |
132 |                 bt.fmt(f)
    |                    ^^^ multiple `fmt` found
    |
    = note: candidate #1 is defined in an impl of the trait `std::fmt::Debug` for the type `backtrace::backtrace::Backtrace`
    = note: candidate #2 is defined in an impl of the trait `std::fmt::Display` for the type `backtrace::backtrace::Backtrace`

error[E0034]: multiple applicable items in scope
   --> src\backtrace\mod.rs:140:20
    |
140 |                 bt.fmt(f)
    |                    ^^^ multiple `fmt` found
    |
    = note: candidate #1 is defined in an impl of the trait `std::fmt::Debug` for the type `backtrace::backtrace::Backtrace`
    = note: candidate #2 is defined in an impl of the trait `std::fmt::Display` for the type `backtrace::backtrace::Backtrace`

error: aborting due to 2 previous errors

Rust was reporting a compilation error saying it was ambiguous which
`fmt` method to call.
@BurntSushi
Copy link

I don't think this is caused by Rust 1.31. failure on master doesn't compile on Rust 1.30 either. My guess is that this PR in backtrace which was just recently merged is the instigator, which I guess made which impl to select ambiguous? Interesting.

@azriel91
Copy link
Author

Ah, I did recently run cargo update in the other project that used this. A very subtle breaking change 👻.

@BurntSushi
Copy link

BurntSushi commented Dec 11, 2018

Indeed. My suspicion is that adding an impl like backtrace did is within the realm of "acceptable" breakage, but yeah, I'm not sure how one anticipates something like this.

cc @alexcrichton

@mpapierski
Copy link

That was fast. Thanks! Looking forward for failure v0.1.4.

@alexcrichton
Copy link

I just boarded a plane and can't deal with this for an hour, I will yank the current version and we can figure how how to add the relevant impl to the backtrace crate later

@mpapierski
Copy link

@alexcrichton That sounds great. Do you mean yank backtrace 0.3.10?

For anyone still puzzled there is temporary workaround:

cargo update -p backtrace --precise 0.3.9 

@alexcrichton
Copy link

Apologies for the breakage here!

For posterity, the root cause was that backtrace 0.3.10 was just published and added Display for Backtrace. This in turn causes the method resolution error in this crate which wanted the Debug implementation.

I deleted Display for Backtrace, published 0.3.11, and yanked 0.3.10. I'll hold off on re-adding Display until a later date, but landing this in the meantime would likely be good regardless!

@RalfJung
Copy link

RalfJung commented Dec 11, 2018

What's up with CI for this PR? In my PR doing the same thing, CI failed because backtrace seems to be no longer compatible with Rust 1.18. Now, this PR seems to have no CI at all.

src/backtrace/mod.rs Outdated Show resolved Hide resolved
azriel91 added a commit to azriel91/failure that referenced this pull request Dec 11, 2018
@azriel91
Copy link
Author

azriel91 commented Dec 11, 2018

@BurntSushi, @alexcrichton, @RalfJung: Since backtrace requires a higher Rust version, what should happen next?

  • Raise the minimum Rust version for failure
  • Cap the version of backtrace in failure
  • Make backtrace Rust 1.18 compatible

First two options affect failure, last option affects backtrace crate (somewhat wider audience)


Edit, just saw rust-lang/backtrace-rs#135, shall claim it.

@azriel91
Copy link
Author

Option 3: rust-lang/backtrace-rs#137

Cargo.toml Outdated Show resolved Hide resolved
@azriel91 azriel91 force-pushed the bugfix/fixed-debug-and-display-impl branch from 392fde7 to 8c3d6a1 Compare December 12, 2018 09:01
azriel91 added a commit to azriel91/backtrace-rs that referenced this pull request Dec 12, 2018
The current latest (0.3.11)` is not compatible with Rust 1.18.0.
Pending <rust-lang/backtrace-rs#137>
@azriel91
Copy link
Author

Pinned backtrace to 0.3.9 temporarily, pending rust-lang/backtrace-rs#137 (and release) before it is Rust 1.18.0 compatible.

Feel free to push over the branch if there's something urgent; I'm off for the night.

azriel91 added a commit to azriel91/backtrace-rs that referenced this pull request Dec 12, 2018
@azriel91
Copy link
Author

azriel91 commented Dec 13, 2018

backtrace 0.3.13 hasn't yet been released, but its minimum supported Rust version is increased to 1.25.0, so I did that here as well.

The `backtrace` crate has this as a minimum required version since
0.3.13.
@azriel91 azriel91 force-pushed the bugfix/fixed-debug-and-display-impl branch from 81d9766 to 16a4e97 Compare December 16, 2018 20:46
@mitsuhiko mitsuhiko closed this in 26fc6eb Dec 28, 2018
@azriel91 azriel91 deleted the bugfix/fixed-debug-and-display-impl branch December 28, 2018 21:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants