Skip to content

Crater Run: Make tyvar_behind_raw_pointer an error #47227

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed

Conversation

mikeyhew
Copy link
Contributor

@mikeyhew mikeyhew commented Jan 6, 2018

In preparation for a crater run to determine its impact

in preparation for a crater run to determine its impact
@rust-highfive
Copy link
Contributor

r? @estebank

(rust_highfive has picked a reviewer for you, use r? to override)

@mikeyhew
Copy link
Contributor Author

mikeyhew commented Jan 6, 2018

r? @nikomatsakis

@shepmaster shepmaster added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 6, 2018
@@ -242,7 +242,7 @@ declare_lint! {

declare_lint! {
pub TYVAR_BEHIND_RAW_POINTER,
Warn,
Deny,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forbid is better for testing existing lints, someone could allow it already.

@mikeyhew
Copy link
Contributor Author

mikeyhew commented Jan 6, 2018

@petrochenkov good point, and some crates could do allow(warnings), right?

@kennytm
Copy link
Member

kennytm commented Jan 6, 2018

@bors try

@bors
Copy link
Collaborator

bors commented Jan 6, 2018

⌛ Trying commit e4a93e4 with merge c5804ea...

bors added a commit that referenced this pull request Jan 6, 2018
…r=<try>

Make tyvar_behind_raw_pointer an error

In preparation for a crater run to determine its impact
@bors
Copy link
Collaborator

bors commented Jan 6, 2018

☀️ Test successful - status-travis
State: approved= try=True

@kennytm
Copy link
Member

kennytm commented Jan 6, 2018

@aidanhs @Mark-Simulacrum crater run wanted.

@shepmaster shepmaster added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 13, 2018
@Mark-Simulacrum
Copy link
Member

Crater run started.

@Mark-Simulacrum
Copy link
Member

http://cargobomb-reports.s3.amazonaws.com/pr-46934/index.html (yes, the pr-# is wrong, but this should be the right run -- if not, let me know, we'll restart and rerun it).

@mikeyhew mikeyhew changed the title Make tyvar_behind_raw_pointer an error Crater Run: Make tyvar_behind_raw_pointer an error Jan 17, 2018
@mikeyhew
Copy link
Contributor Author

60 regressions – is that good? It's better than I expected, anyway. And is it safe to say that all of the "fixed" crates and any test-fail regressions were spurious?

@nikomatsakis what do the results tell you?

@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Jan 17, 2018
@nikomatsakis
Copy link
Contributor

60 seems like a lot to me =) but it all depends on the details. Let me dig in a bit.

@nikomatsakis
Copy link
Contributor

I got as far as link-0.1.0. Most of the results I've looked at look like legit, distinct regressions. Here are the ones I see that are spurious:

  • cc-1.0.4
  • conc-0.5.0
  • crux-0.3.0
  • ctx-0.2.0
  • esprit-0.0.5
  • gear-0.1.3
  • hashconsing-0.3.0
  • hornet-0.1.0
  • kernel_density-0.0.1
  • libfuzzy-sys-0.1.0

This merits some discussion and consideration, I think. Of course, the future compatibility warning may lead some people to migrate, but we may want to think about some special method resolution rules -- for example, giving inherent methods defined on *mut T some sort of special precedence.

@nikomatsakis
Copy link
Contributor

Well, I think this PR has served its purpose, right? I'm going to close for now. (It's not meant to be merged, I don't think.)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants