-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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 checking for missing stability annotations #43270
Conversation
Remove couple of unnecessary `#![feature(staged_api)]`.
@@ -313,8 +313,9 @@ struct MissingStabilityAnnotations<'a, 'tcx: 'a> { | |||
impl<'a, 'tcx: 'a> MissingStabilityAnnotations<'a, 'tcx> { | |||
fn check_missing_stability(&self, id: NodeId, span: Span) { | |||
let def_id = self.tcx.hir.local_def_id(id); | |||
let stab = self.tcx.stability.borrow().stab_map.get(&def_id).cloned(); |
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.
While I doubt this has any serious implications, the cloned
here seems unnecessary since AFAICT we never use anything but (one of) the None
variants inside. Could we remove it?
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.
It clones a reference to Stability
, not Stability
itself. cloned
is necessary to avoid borrow checking error with a short living temporary.
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.
Ah, so just to clarify this goes from &&T
to &T
? Unfortunate that it causes borrow checking problems. Perhaps map_or
could help?
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.
Ah, so just to clarify this goes from &&T to &T?
Yes.
@bors: r+ |
📌 Commit 465ada6 has been approved by |
⌛ Testing commit 465ada6 with merge c0197fc9ba050444dde555c7dc1ab6421ebcfb81... |
💔 Test failed - status-travis |
@bors retry |
⌛ Testing commit 465ada6 with merge 4dd8fc7022179c93bd79f9666927d27f6e29898c... |
💔 Test failed - status-appveyor |
@bors: retry
…On Wed, Jul 19, 2017 at 4:06 AM, bors ***@***.***> wrote:
💔 Test failed - status-appveyor
<https://ci.appveyor.com/project/rust-lang/rust/build/1.0.3973>
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#43270 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAD95L6656Y8kkM8-TbmTw_VLsFdHigaks5sPcclgaJpZM4OZZd8>
.
|
Fix checking for missing stability annotations This was a regression from #37676 causing "unmarked API" ICEs like #43027. r? @alexcrichton
☀️ Test successful - status-appveyor, status-travis |
This was a regression from #37676 causing "unmarked API" ICEs like #43027.
r? @alexcrichton