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

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

Conversation

Projects
None yet
5 participants
@azriel91
Copy link

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
Use associated function syntax for `Debug` and `Display` backtrace impl
Rust was reporting a compilation error saying it was ambiguous which
`fmt` method to call.
@BurntSushi

This comment has been minimized.

Copy link
Member

BurntSushi commented Dec 11, 2018

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

This comment has been minimized.

Copy link
Author

azriel91 commented Dec 11, 2018

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

@BurntSushi

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link

mpapierski commented Dec 11, 2018

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

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Dec 11, 2018

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

This comment has been minimized.

Copy link

mpapierski commented Dec 11, 2018

@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 

@eoger eoger referenced this pull request Dec 11, 2018

Merged

Rust 2018 #455

@mindriot101 mindriot101 referenced this pull request Dec 11, 2018

Merged

Fix build problem #100

@phansch phansch referenced this pull request Dec 11, 2018

Merged

rustfmt tests #3529

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Dec 11, 2018

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

This comment has been minimized.

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.

Show resolved Hide resolved src/backtrace/mod.rs Outdated

azriel91 added a commit to azriel91/failure that referenced this pull request Dec 11, 2018

@azriel91

This comment has been minimized.

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 alexcrichton/backtrace-rs#135, shall claim it.

@azriel91

This comment has been minimized.

Copy link
Author

azriel91 commented Dec 12, 2018

Show resolved Hide resolved Cargo.toml Outdated

@azriel91 azriel91 force-pushed the azriel91:bugfix/fixed-debug-and-display-impl branch from 392fde7 to 8c3d6a1 Dec 12, 2018

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

Lock `backtrace` crate to 0.3.9.
The current latest (0.3.11)` is not compatible with Rust 1.18.0.
Pending <alexcrichton/backtrace-rs#137>
@azriel91

This comment has been minimized.

Copy link
Author

azriel91 commented Dec 12, 2018

Pinned backtrace to 0.3.9 temporarily, pending alexcrichton/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

This comment has been minimized.

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.

Raise minimum supported Rust version to 1.25.0.
The `backtrace` crate has this as a minimum required version since
0.3.13.

@azriel91 azriel91 force-pushed the azriel91:bugfix/fixed-debug-and-display-impl branch from 81d9766 to 16a4e97 Dec 16, 2018

@mitsuhiko mitsuhiko closed this in 26fc6eb Dec 28, 2018

@azriel91 azriel91 deleted the azriel91:bugfix/fixed-debug-and-display-impl branch Dec 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.