-
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 compile panic on non existent type return #53460
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @estebank (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
@@ -25,41 +25,43 @@ | |||
//! crate as a kind of pass. This should eventually be factored away. | |||
|
|||
use astconv::{AstConv, Bounds}; | |||
use lint; |
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.
Please separate the rustfmt changes into their own commit
src/test/compile-fail/issue-53300.rs
Outdated
@@ -0,0 +1,22 @@ | |||
// Copyright 2012-2014 The Rust Project Developers. See the COPYRIGHT |
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.
Can you turn this into an ui
test instead? We're slowly moving away from compile-fail
. For an example as to why, the span bug generated is not visible in this test because it has a DUMMY_SP
.
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.
K, will do. Thanks. 👍
☔ The latest upstream changes (presumably #52953) made this pull request unmergeable. Please resolve the merge conflicts. |
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 |
@JoshBrudnak please don't use merge, use rebase for clean patches instead. Also, the merge failed due to #52953:
|
Rebased to fix the merge conflict. |
@bors r+ p=10 (will likely cause merge conflicts due to rustfmt) |
📌 Commit 264d0a2 has been approved by |
Fix compile panic on non existent type return Reverted the change 28a76a9#diff-4ed25c00aceb84666fca639cf8101c7cL1069 which was panicking when returning a type that cannot be found in the current scope and added testing for the compile error. For example: ```rust fn addition() -> Wrapper<impl A> {} ``` Where Wrapper is undefined in the scope.
☀️ Test successful - status-appveyor, status-travis |
Reverted the change 28a76a9#diff-4ed25c00aceb84666fca639cf8101c7cL1069 which was panicking when returning a type that cannot be found in the current scope and added testing for the compile error.
For example:
Where Wrapper is undefined in the scope.