-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
ICE: trimmed_def_paths constructed, assertion failed: !new_self_ty.has_escaping_bound_vars() #96223
Comments
Any way you could run this with a compiler with debug info included? Line numbers would be helpful for the stacktrace. It looks like there's a bug in the error reporting path. |
@jackh726 as in, build one locally? yeah can do |
Yes :) |
Actually, probably not needed. This code is wonky:
|
Probably the best way to fix this would be to make I might or might not be able to find the time to work on this, but it's pretty easy and I can mentor someone. There's also quite a bit of code that calls |
I think the more time-intensive part of this bug is getting a test case 😉 |
Also cc @fee1-dead who introduced this in fdf7d01#diff-aef5d12ac7ec2060165cf1db78128f941c789bb2fc90c7ed351049b0be4d435eL727 |
Got you your backtrace, anyway
|
Just got back from vacation. I can work on this sometime this week :^) @rustbot claim |
(Given that this is also an easy change, I would be tempted to "leave" this for someone with minimal experience that wants to get involved. It's a good beginner issue.) |
Good point. I can definitely find other things to work on. @rustbot release-assignment |
First attempt of contribution, so let's go 😄 @rustbot claim |
n.b. I would like to get this backported into the beta in time for a release, if possible, this breaks code in ways that I'm not sure can be worked around. cc @rust-lang/release Going to try and see if I can come up with a reduced testcase. |
I got it somewhat reduced to an independent rust file: use std::marker::PhantomData;
pub trait Deserialize<'de>: Sized {
// fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
// where
// D: Deserializer<'de>;
}
pub trait Yokeable<'a>: 'static {
type Output: 'a;
}
pub trait DataMarker {
type Yokeable: for<'a> Yokeable<'a>;
}
pub struct YokeTraitHack<T>(pub T);
impl<'de, T> Deserialize<'de> for YokeTraitHack<T>
where
T: Deserialize<'de> {}
struct FsDataProvider;
impl<M> DynProvider<M> for FsDataProvider
where
M: DataMarker,
for<'de> YokeTraitHack<<M::Yokeable as Yokeable<'de>>::Output>: Deserialize<'de> {
}
// pub struct DataPayload<M: DataMarker>(<M::Yokeable as Yokeable<'static>>::Output);
pub trait DynProvider<M>
where
M: DataMarker,
{
// fn load_payload(
// &self,
// key: &str
// ) -> DataPayload<M>;
}
pub struct CodePointSet<'a>(&'a [u8]);
impl<'a> Yokeable<'a> for CodePointSet<'static> {
type Output = CodePointSet<'a>;
}
pub struct CodePointSetMarker;
impl DataMarker for CodePointSetMarker {
type Yokeable = CodePointSet<'static>;
}
fn icey_bounds<D: DynProvider<CodePointSetMarker>>(p: &D) {
// let ascii_hex_digit = get_ascii_hex_digit(p).expect("hi");
}
fn trigger_ice() {
let p = FsDataProvider;
icey_bounds(&p);
}
fn main() {} |
Adding an |
As a quick test hack: if one bails out of the suggestion when there are escaping bound vars, the ICE would be "fixed" and this code would emit "error[E0277]: the trait bound |
I'm a bit mixed on whether we should backport a fix here. One on hand, the actual fix here should be very small and relatively safe (it only occurs in the error path). On the other hand, this doesn't break working code. @ricked-twice had some changes that were pretty minimal (returning if @ricked-twice how does this sound to you? If you'd like to open a PR with the original changes you had, that would be a good candidate to backport. Then you can followup with changing |
Yeah, sorry, I made that comment before I realized that there was indeed a fix for the issue 😄 I don't think we need to backport as strongly. But I do feel the smaller bugfix would be worth attempting to backport. |
Like I said, I think the backport is relatively safe. But not sure it motivates a point release on its own. We can start with the small PR and let compiler team and release team decide how to proceed. |
Right right, when I said backport I mostly meant backporting to beta (which stabilizes on May 16ish) |
@ricked-twice is there anything I can help with here? |
Sorry I was not available last week ! But, yes, I'm still working on it. As I was suggested to, a deeper change to By the way, @Manishearth, thank you for the minimal test case. I was trying to get one, but it was kind of hard for me to get through your code and understand correctly the ICE triggering part. |
Yeah I think we can land the simple fix with the testcase at least. (Maybe still worth backporting; a coworker is hitting this bug in a context where it's not clear if there's a missing trait) |
…kh726 Quick fix for rust-lang#96223. This PR is a quick fix regarding rust-lang#96223. As mentioned in the issue, others modification could be added to not elide types with bound vars from suggestions. Special thanks to `@jackh726` for mentoring and `@Manishearth` for minimal test case. r? `@jackh726`
…kh726 Quick fix for rust-lang#96223. This PR is a quick fix regarding rust-lang#96223. As mentioned in the issue, others modification could be added to not elide types with bound vars from suggestions. Special thanks to ``@jackh726`` for mentoring and ``@Manishearth`` for minimal test case. r? ``@jackh726``
Rollup of 6 pull requests Successful merges: - rust-lang#96597 (openbsd: unbreak build on native platform) - rust-lang#96662 (Fix typo in lint levels doc) - rust-lang#96668 (Fix flaky rustdoc-ui test because it did not replace time result) - rust-lang#96679 (Quick fix for rust-lang#96223.) - rust-lang#96684 (Update `ProjectionElem::Downcast` documentation) - rust-lang#96686 (Add some TAIT-related tests) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Fixed by #96679 |
This should stay open until that PR is backported. |
…ulacrum [beta] backports This backports/rolls up: * Quick fix for rust-lang#96223. rust-lang#96679 * [beta] Revert rust-lang#92519 on beta rust-lang#96556 * [beta] Clippy backport ICE/infinite loop fix rust-lang#96740 * Revert "Prefer projection candidates instead of param_env candidates for Sized predicates" rust-lang#96593
- Modified `InferCtxt::mk_trait_obligation_with_new_self_ty` to take as argument a `Binder<(TraitPredicate, Ty)>` instead of a `Binder<TraitPredicate>` and a separate `Ty` with no bound vars. - Modified all call places to avoid calling `Binder::no_bounds_var` or `Binder::skip_binder` when it is not safe.
Rollup of 6 pull requests Successful merges: - rust-lang#94639 (Suggest dereferencing non-lval mutable reference on assignment) - rust-lang#95979 (update coherence docs, fix generator + opaque type ICE) - rust-lang#96378 (Mention traits and types involved in unstable trait upcasting) - rust-lang#96917 (Make HashMap fall back to RtlGenRandom if BCryptGenRandom fails) - rust-lang#97101 (Add tracking issue for ExitCode::exit_process) - rust-lang#97123 (Clean fix for rust-lang#96223) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Code
Unfortunately I'm unable to reduce the testcase, but you can run it by opening https://github.com/Manishearth/icu4x/tree/ice-trimmed-def-paths. and from the
components/properties
directory, runningcargo test --doc ascii_hex_digit
. This code is in the midst of a refactor so most of the other tests will not pass.I suspect it has to do with the complicated trait stuff going on in provider.rs interacting with the docs.rs/yoke crate (cc @jackh726 @eddyb) but I'm not sure.
Meta
Bug occurs on 1.60 stable and current nightly.
rustc --version --verbose
:Error output
Backtrace
The text was updated successfully, but these errors were encountered: