update error generic member access

[waynr]

The purpose of this PR is to align thiserror with changes being made to the provide_any and error_generic_member_access features in nightly Rust, specifically:

• The Provider trait of the provide_any feature has been rejected.
• What's left of the provide_any (namely, the Demand/Request type) RFC has been effectively merged into the error_generic_member_access RFC

I've verified this PR builds as expected and the tests run with both the stable and nightly toolchains as well as a custom-built toolchain using my draft PR that removes the Provider trait from nightly rust. The only thing I've found doesn't work with the current nightly toolchain are the tests in tests/test_backtrace.rs (because those depend on the rename of Request to Demand).

Builds continue to compile for nightly because of the compile probe in build.rs that only sets the error_generic_member_access if the probe code compiles, which it won't until my draft PR to rust is merged. This could potentially be a problem for thiserror nightly users who have come to rely on the backtrace functionality when provide_any/error_generic_member_access is enabled.

I have thought a bit about merge strategy considerations here since there are rust tools that (in nightly rust) rely on thiserror which won't build without the changes in this PR (or at least I haven't figured out a way to get them to build yet). Because of this chicken-and-egg problem and because thiserror continues to build on nightly branch (albeit without the nifty generic member access to backtraces), my suggestion is to merge this change prior to my draft PR to rust. The only alternative I can think of for that PR is to first pin all rust dependencies and transitive thiserror dependencies to the git commit in this PR; but I don't think that would be very palatable for rust maintainers.

[waynr]

For what it's worth, the test failure on this PR is what I expected based on my attempts to use a nightly toolchain when running the tests locally. I wouldn't expect those tests to pass until rust-lang/rust#113464 is merged.

[dtolnay]

Thanks for the PR. Is rust-lang/rust#113464 (comment) the failure that this is intended to address? That error is caused by a bug in rustc's bootstrap where, in some stages, the standard library that gets picked up if $RUSTC is run from a build script is not the standard library that the rest of the crate will get built against.

That needs to get fixed in bootstrap.

I would prefer not to accept this PR because it implies that the iteration loop on rust-lang/rust#113464 involves creating a thiserror PR, reviewing/merging the PR, publishing a release of thiserror, upgrading thiserror in rustc's Cargo.lock, and then updating the standard library PR, which is not reasonable.

I would be open to a different thiserror PR that does not rewrite any of the macro, but just changes build.rs to detect when it is being built as a dependency of rustc (hopefully there is an environment variable or cfg for this) and permanently disables unstable functionality in that case. Rustc tools would not be able to get backtraces from thiserror even after the Provider refactor, until either bootstrap's handling of the build script environment is fixed or the generic member access API stabilizes.

[waynr]

First off, thanks so much for the quick feedback!

Thanks for the PR. Is rust-lang/rust#113464 (comment) the failure that this is intended to address?

Yes, in part. But strangely enough I can't seem to reproduce the issue 🤔 -- x doc now succeeds for me. Maybe something about my build configuration changed without my knowledge (I'm definitely still new to building rust) between a few weeks ago and now.

I also wanted to prep this PR as a courtesy since I am proposing a change to the nightly rust that will break nightly builds of thiserror. There's also the fact that I'm a frequent user of this crate and I like looking through and contributing to code that I use.

I would prefer not to accept this PR because it implies that the iteration loop on rust-lang/rust#113464 involves creating a thiserror PR, reviewing/merging the PR, publishing a release of thiserror, upgrading thiserror in rustc's Cargo.lock, and then updating the standard library PR, which is not reasonable.

Okay, let's say I figure out how to reproduce the bootstrap bug you linked to in my PR comment, I fix that, and my rust PR eventually gets approved and merged -- after that, would you want something like this PR?

I would be open to a different thiserror PR that does not rewrite any of the macro,

Does that mean you want to keep src/provide.rs? The git history on that file suggested to me that it only exists to reconcile the method name conflict between Provider and Error in the current rust nightly builds. The current state of feedback from the rust libs meeting is that they don't want to include a Provider trait at all. So while my rust PR might not be the final stabilized version of the error_generic_member_access feature and is likely to have changes requested of it before it's merged, at least that one element of the feature is almost certain not to be included. That's why I removed the ThiserrorProvide trait and its usage in the macro -- the only other macro change I made was to rename Demand to Request (a name change which I admit is still subject to a bit of ongoing bikeshedding).

Anyway, I'll ask on Zulip if there is any way to detect that code is being compiled as part of a rust toolchain build so we can do as you suggest. Thanks again for the feedback!

[dtolnay]

my rust PR eventually gets approved and merged -- after that, would you want something like this PR?

That's right — after the changed API is in nightly, I can reopen this (and the CI will pass).

[waynr]

my rust PR eventually gets approved and merged -- after that, would you want something like this PR?

That's right — after the changed API is in nightly, I can reopen this (and the CI will pass).

Okay cool.

Also, just FYI -- I realized why I can no longer reproduce that bootstrap bug. It's because one of the latest commits on my rust PR actually removes the provide_any feature flag. I just haven't been working on this consistently enough to remember that I did that 🤦