-
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
Fix #57730 #57981
Fix #57730 #57981
Conversation
use std::borrow::Borrow; | ||
use core::fmt::Debug; | ||
|
||
fn test(_: impl Borrow<impl Debug>) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should definitely not be allowed as written here; There's even a comment saying so in https://github.com/rust-lang/rust/pull/57730/files#diff-1b4892578c33275e3b1ff1fe43b7f2f9.
Instead, this should be moved to test/ui/impl-trait/where.rs
as an example of what isn't allowed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see there's already tests for that. I just removed this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well not exactly impl A<impl B>
... it would be nice to have such a test.
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@Zoxc This is now fixing a PR, not an issue... |
It fixes a bug introduced in the PR. Technically it fixes #57979 too, but we may want to keep that open until we actually error on the example there. |
Per my comment here, this could was never actually meant to be accepted. I haven't read the details of this PR yet, so I'm not sure precisely what we should do make it start erroring again -- or maybe we want a future compatibility warning. |
This PR just fixed a bug which causes part of AST validation to be skipped. We definitely want to merge this. |
@Zoxc Ok; Can you add tests ensuring that #57979 (comment) (in return position also) doesn't compile? |
@Centril I'm not sure what you mean. That does compile. |
📌 Commit e6d5f25 has been approved by |
Hmm, according to my local testing, we still see the expected error here, even with this patch applied. I'm pushing a test case, let's see what travis thinks. |
1ec3ea2
to
9d2a0b9
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
r=me if travis is happy. As discussed in the compiler meeting, this PR seems to fix an obvious bug, but I don't think that it actually affects the behavior of #57979 at all. I added a regression test for the moment that documents the behavior in that case, since apparently it was untested. Locally, that test passes for me. |
@bors r=nikomatsakis |
📌 Commit cce2c89 has been approved by |
Rollup of 19 pull requests Successful merges: - #57929 (Rustdoc remove old style files) - #57981 (Fix #57730) - #58074 (Stabilize slice_sort_by_cached_key) - #58196 (Add specific feature gate error for const-unstable features) - #58293 (Remove code for updating copyright years in generate-deriving-span-tests) - #58306 (Don't default on std crate when manipulating browser history) - #58359 (librustc_mir: use ? in impl_snapshot_for! macro) - #58395 (Instant::checked_duration_since) - #58429 (fix Box::into_unique effecitvely transmuting to a raw ptr) - #58433 (Update which libcore/liballoc tests Miri ignores, and document why) - #58438 (Use posix_spawn_file_actions_addchdir_np when possible) - #58440 (Whitelist the ARM v6 target-feature) - #58448 (rustdoc: mask `compiler_builtins` docs) - #58468 (split MaybeUninit into several features, expand docs a bit) - #58477 (Fix the syntax error in publish_toolstate.py) - #58479 (compile-pass test for #53606) - #58489 (Fix runtime error in generate-keyword-tests) - #58496 (Fix documentation for std::path::PathBuf::pop) - #58509 (Notify myself when Clippy toolstate changes)
Rollup of 19 pull requests Successful merges: - #57929 (Rustdoc remove old style files) - #57981 (Fix #57730) - #58074 (Stabilize slice_sort_by_cached_key) - #58196 (Add specific feature gate error for const-unstable features) - #58293 (Remove code for updating copyright years in generate-deriving-span-tests) - #58306 (Don't default on std crate when manipulating browser history) - #58359 (librustc_mir: use ? in impl_snapshot_for! macro) - #58395 (Instant::checked_duration_since) - #58429 (fix Box::into_unique effecitvely transmuting to a raw ptr) - #58433 (Update which libcore/liballoc tests Miri ignores, and document why) - #58438 (Use posix_spawn_file_actions_addchdir_np when possible) - #58440 (Whitelist the ARM v6 target-feature) - #58448 (rustdoc: mask `compiler_builtins` docs) - #58468 (split MaybeUninit into several features, expand docs a bit) - #58479 (compile-pass test for #53606) - #58489 (Fix runtime error in generate-keyword-tests) - #58496 (Fix documentation for std::path::PathBuf::pop) - #58509 (Notify myself when Clippy toolstate changes) - #58521 (Fix tracking issue for error iterators)
…mpl-trait, r=zoxc Warning period for detecting nested impl trait Here is some proposed code for making a warning period for the new checking of nested impl trait. It undoes some of the corrective effects of PR #57730, by using boolean flags to track parts of the analysis that were previously skipped prior to PRs #57730 and #57981 landing. Cc #57979
cc #57730
r? @cramertj