Skip to content
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

Don't leak the compiler's internal representation of scopes in error messages. #38552

Merged
merged 2 commits into from
Jan 4, 2017

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Dec 22, 2016

Fixes #37884 (actually fixes #27942, which was made worse by #37412) by handling more node types.
Ideally we'd turn the unknown node type situations into ICEs and fix them as they show up in errors.
But we might want to backport this patch so I was less aggressive.

@eddyb eddyb added beta-nominated Nominated for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 22, 2016
@eddyb eddyb requested a review from nikomatsakis December 22, 2016 17:50
@rust-highfive
Copy link
Collaborator

r? @pnkfelix

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

@arielb1
Copy link
Contributor

arielb1 commented Dec 22, 2016

@bors r+

@bors
Copy link
Contributor

bors commented Dec 22, 2016

📌 Commit 25b1d2c has been approved by arielb1

@nikomatsakis nikomatsakis added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Dec 22, 2016
@nikomatsakis
Copy link
Contributor

Marked as beta-accepted: regr in diagnostics, low risk change.

@alexcrichton
Copy link
Member

@bors: p=1

(beta nominated)

@bors
Copy link
Contributor

bors commented Dec 27, 2016

⌛ Testing commit 25b1d2c with merge a1708c2...

@bors
Copy link
Contributor

bors commented Dec 27, 2016

💔 Test failed - status-travis

@nikomatsakis nikomatsakis self-assigned this Dec 27, 2016
@alexcrichton
Copy link
Member

alexcrichton commented Dec 27, 2016 via email

@bors
Copy link
Contributor

bors commented Dec 27, 2016

⌛ Testing commit 25b1d2c with merge 9fc4005...

@bors
Copy link
Contributor

bors commented Dec 27, 2016

💔 Test failed - status-travis

@eddyb
Copy link
Member Author

eddyb commented Dec 27, 2016

How can that even happen? It almost sounds like one line was replaced with the previous line, musl bug?

@eddyb
Copy link
Member Author

eddyb commented Dec 30, 2016

@bors retry

@bors
Copy link
Contributor

bors commented Dec 30, 2016

⌛ Testing commit 25b1d2c with merge abd4aa7...

@bors
Copy link
Contributor

bors commented Dec 30, 2016

💔 Test failed - status-travis

@nikomatsakis
Copy link
Contributor

@eddyb

looks legitimate

   Compiling rustc v0.0.0 (file:///checkout/src/librustc)

error[E0531]: unresolved tuple struct/variant `hir::MethodTraitItem`

   --> /checkout/src/librustc/infer/error_reporting.rs:118:17

    |

118 |                 hir::MethodTraitItem(..) => "method body",

    |                 ^^^^^^^^^^^^^^^^^^^^

error[E0531]: unresolved tuple struct/variant `hir::ConstTraitItem`

   --> /checkout/src/librustc/infer/error_reporting.rs:119:17

    |

119 |                 hir::ConstTraitItem(..) |

    |                 ^^^^^^^^^^^^^^^^^^^

error[E0531]: unresolved tuple struct/variant `hir::TypeTraitItem`

   --> /checkout/src/librustc/infer/error_reporting.rs:120:17

    |

120 |                 hir::TypeTraitItem(..) => "associated item"

    |                 ^^^^^^^^^^^^^^^^^^

error: aborting due to 3 previous errors

@eddyb
Copy link
Member Author

eddyb commented Jan 4, 2017

I think I'll keep changes that are needed solely to make the original patch on master, separate.
At least then the original commit should still apply on beta.

@eddyb
Copy link
Member Author

eddyb commented Jan 4, 2017

@bors r=arielb1

@bors
Copy link
Contributor

bors commented Jan 4, 2017

📌 Commit 987f52f has been approved by arielb1

@bors
Copy link
Contributor

bors commented Jan 4, 2017

⌛ Testing commit 987f52f with merge e06ce71...

bors added a commit that referenced this pull request Jan 4, 2017
Don't leak the compiler's internal representation of scopes in error messages.

Fixes #37884 (actually fixes #27942, which was made worse by #37412) by handling more node types.
Ideally we'd turn the unknown node type situations into ICEs and fix them as they show up in errors.
But we might want to backport this patch so I was less aggressive.
@bors
Copy link
Contributor

bors commented Jan 4, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: arielb1
Pushing e06ce71 to master...

@bors bors merged commit 987f52f into rust-lang:master Jan 4, 2017
@eddyb eddyb deleted the bad-blocks branch January 4, 2017 10:38
@alexcrichton
Copy link
Member

@eddyb this PR backports cleanly to beta but then results in a build failure. Can you investigate backporting to beta?

@eddyb
Copy link
Member Author

eddyb commented Jan 6, 2017

@alexcrichton Do you have the build failure?

@alexcrichton
Copy link
Member

@eddyb unfortunately no

@nikomatsakis nikomatsakis mentioned this pull request Jan 6, 2017
17 tasks
@alexcrichton alexcrichton removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jan 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
7 participants