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.

Sign up for GitHub

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

Jump to bottom

Lower constant patterns with ascribed types. #58161

Merged
merged 1 commit into from
Feb 8, 2019
Merged

Lower constant patterns with ascribed types. #58161

merged 1 commit into from
Feb 8, 2019

Conversation

davidtwco
Copy link
Member

Fixes #57960.

This PR fixes a bug introduced by #55937 which started checking user
type annotations for associated type patterns. Where lowering a
associated constant expression would previously return a
PatternKind::Constant, it now returns a PatternKind::AscribeUserType
with a PatternKind::Constant inside, this PR unwraps that to
access the constant pattern inside and behaves as before.

r? @pnkfelix

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 4, 2019
@davidtwco davidtwco mentioned this pull request Feb 4, 2019
Closed
@arielb1
Copy link
Contributor

arielb1 commented Feb 6, 2019

r=me with comment in place

@davidtwco
Lower constant patterns with ascribed types.
6717727 
This commit fixes a bug introduced by rust-lang#55937 which started checking user
type annotations for associated type patterns. Where lowering a
associated constant expression would previously return a
`PatternKind::Constant`, it now returns a `PatternKind::AscribeUserType`
with a `PatternKind::Constant` inside, this commit unwraps that to
access the constant pattern inside and behaves as before.
@arielb1
Copy link
Contributor

arielb1 commented Feb 6, 2019

So @matthewjasper had complained that the AscribeUserType was getting dropped. Could you add it back, along with a mir-opt test?

r=me with that

@arielb1
Copy link
Contributor

arielb1 commented Feb 8, 2019

It appears that fixing the AscribeUserType problem is more complicated than it appears at first glance - I have split it into #58299 - but we need to fix this PR for beta.

@bors r+ p=1

@bors
Copy link
Contributor

bors commented Feb 8, 2019

📌 Commit 6717727 has been approved by arielb1

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 8, 2019
@arielb1 arielb1 added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Feb 8, 2019
@arielb1
Copy link
Contributor

arielb1 commented Feb 8, 2019

beta-nominating because this fixes a regression in beta.

@arielb1 arielb1 added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 8, 2019
@bors
Copy link
Contributor

bors commented Feb 8, 2019

⌛ Testing commit 6717727 with merge 7fcc7a28ebc267130f91107187c3073658f8482e...

@bors
Copy link
Contributor

bors commented Feb 8, 2019

💔 Test failed - checks-travis

@rust-highfive
Copy link
Collaborator

The job dist-x86_64-apple of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log. 
[00:01:34]    Compiling serde_derive v1.0.81
[00:01:37]    Compiling toml v0.4.10
[00:01:37]    Compiling serde_json v1.0.33
[00:01:47]    Compiling bootstrap v0.0.0 (/Users/travis/build/rust-lang/rust/src/bootstrap)
No output has been received in the last 30m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

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 @TimNN. (Feature Requests)

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 8, 2019
@arielb1
Copy link
Contributor

arielb1 commented Feb 8, 2019

No output has been received in the last 30m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 8, 2019
@arielb1
Copy link
Contributor

arielb1 commented Feb 8, 2019

cc @kennytm looks spurious

bors added a commit that referenced this pull request Feb 8, 2019
@bors
Auto merge of #58161 - davidtwco:issue-57960, r=arielb1
a2ec156 
Lower constant patterns with ascribed types.

Fixes #57960.

This PR fixes a bug introduced by #55937 which started checking user
type annotations for associated type patterns. Where lowering a
associated constant expression would previously return a
`PatternKind::Constant`, it now returns a `PatternKind::AscribeUserType`
with a `PatternKind::Constant` inside, this PR unwraps that to
access the constant pattern inside and behaves as before.

r? @pnkfelix
@bors
Copy link
Contributor

bors commented Feb 8, 2019

⌛ Testing commit 6717727 with merge a2ec156...

@bors
Copy link
Contributor

bors commented Feb 8, 2019

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 8, 2019
@bors bors merged commit 6717727 into rust-lang:master Feb 8, 2019
@davidtwco davidtwco deleted the issue-57960 branch February 10, 2019 11:23
@davidtwco davidtwco mentioned this pull request Feb 11, 2019
Merged
davidtwco added a commit to davidtwco/rust that referenced this pull request Feb 12, 2019
@davidtwco
Check user type annotations for range patterns.
ee82d09 
This commit builds on the fix from rust-lang#58161 (which fixed miscompilation
caused by the introduction of `AscribeUserType` patterns for associated
constants) to start checking these patterns are well-formed for ranges
(previous fix just ignored them so that miscompilation wouldn't occur).
Centril added a commit to Centril/rust that referenced this pull request Feb 14, 2019
@Centril
Rollup merge of rust-lang#58371 - davidtwco:issue-58299, r=arielb1
0dbb867 
Check user type annotations for range patterns.

Fixes rust-lang#58299.

This PR builds on the fix from rust-lang#58161 (which fixed miscompilation
caused by the introduction of `AscribeUserType` patterns for associated
constants) to start checking these patterns are well-formed for ranges
(previous fix just ignored them so that miscompilation wouldn't occur).

r? @arielb1
@pnkfelix
Copy link
Member

triage, beta-accepted.

@pnkfelix pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Feb 14, 2019
@pietroalbini pietroalbini mentioned this pull request Feb 17, 2019
Merged
@pietroalbini pietroalbini removed beta-nominated Nominated for backporting to the compiler in the beta channel. labels Feb 17, 2019
bors added a commit that referenced this pull request Feb 17, 2019
@bors
Auto merge of #58537 - pietroalbini:beta-backports, r=pietroalbini
1456495 
[beta] Rollup backports

Cherry-picked:

* #58207: Make `intern_lazy_const` actually intern its argument.
* #58161: Lower constant patterns with ascribed types.
* #57908: resolve: Fix span arithmetics in the import conflict error
* #57835: typeck: remove leaky nested probe during trait object method resolution
* #57885: Avoid committing to autoderef in object method probing
* #57646: Fixes text becoming invisible when element targetted

Rolled up:

* #58522: [BETA] Update cargo

r? @ghost
bors added a commit that referenced this pull request Feb 20, 2019
@bors
Auto merge of #58537 - pietroalbini:beta-backports, r=pietroalbini
1f84156 
[beta] Rollup backports

Cherry-picked:

* #58207: Make `intern_lazy_const` actually intern its argument.
* #58161: Lower constant patterns with ascribed types.
* #57908: resolve: Fix span arithmetics in the import conflict error
* #57835: typeck: remove leaky nested probe during trait object method resolution
* #57885: Avoid committing to autoderef in object method probing
* #57646: Fixes text becoming invisible when element targetted

Rolled up:

* #58522: [BETA] Update cargo

r? @ghost
@kennytm kennytm mentioned this pull request Feb 22, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewjasper matthewjasper matthewjasper left review comments

@arielb1 arielb1 arielb1 left review comments

Assignees

@pnkfelix pnkfelix

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Associated-const ranges in patterns are miscompiled in beta
7 participants
@davidtwco @arielb1 @bors @rust-highfive @pnkfelix @matthewjasper @pietroalbini