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

Fix ICE #3719+#3718 in lint match_ref_pats #3772

Merged
merged 4 commits into from
Feb 25, 2019
Merged

Conversation

flip1995
Copy link
Member

@flip1995 flip1995 commented Feb 16, 2019

Fixes #3719
This conveniently also fixes #3718

The ICE occurs when the match expression was a macro call, where the macro was defined in another file. Since we don't have the ability to reproduce this behavior with our UI tests (AFAIK), I couldn't add a test reproducing this ICE.. However, I added a test which is related to the ICE, to show the new behavior of the lint.

I tested it with the mscheme repo locally and the ICE didn't happen anymore.

r? @matthiaskrgr

@Manishearth
Copy link
Member

Is it not possible to declare aux dependency files in UI tests? Compiletest has this ability, grep the rustc tests for "aux"

@flip1995
Copy link
Member Author

flip1995 commented Feb 18, 2019

Yeah I tried this, but this didn't led to the ICE. The aux-build flag of compiletest-rs somehow includes the span of the macro into the test file (or something like this). The suggestion looked similar to this:

LL | // aux-build:aux.rs*Foo :: get(0) . unwrap() 

This test doesn't reproduce the ICE since it only happens, when the macro is defined in another file.
Currently we can't add tests with multiple files AFAIK

Also using the auxiliary folder didn't help
@flip1995 flip1995 changed the title Fix ICE #3719 in lint match_ref_pats Fix ICE #3719+#3718 in lint match_ref_pats Feb 19, 2019
@flip1995 flip1995 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 20, 2019
@Manishearth
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Feb 25, 2019

📌 Commit 75f3988 has been approved by Manishearth

bors added a commit that referenced this pull request Feb 25, 2019
Fix ICE #3719+#3718 in lint match_ref_pats

Fixes #3719
This conveniently also fixes #3718

The ICE occurs when the match expression was a macro call, where the macro was defined in another file. Since we don't have the ability to reproduce this behavior with our UI tests (AFAIK), I couldn't add a test reproducing this ICE.. However, I added a test which is related to the ICE, to show the new behavior of the lint.

I tested it with the mscheme repo locally and the ICE didn't happen anymore.

r? @matthiaskrgr
@bors
Copy link
Collaborator

bors commented Feb 25, 2019

⌛ Testing commit 75f3988 with merge 1ac6f4e...

@bors
Copy link
Collaborator

bors commented Feb 25, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: Manishearth
Pushing 1ac6f4e to master...

@bors bors merged commit 75f3988 into rust-lang:master Feb 25, 2019
@flip1995 flip1995 deleted the ice-3719 branch February 25, 2019 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

ICE in https://github.com/meddle0x53/mscheme/ ICE in https://github.com/stainless-steel/sql
3 participants