Skip to content

Make novel structural match violations not a bug - #73446

Merged
bors merged 3 commits into
rust-lang:masterfrom
ecstatic-morse:issue-73431
Jun 19, 2020
Merged

Make novel structural match violations not a bug#73446
bors merged 3 commits into
rust-lang:masterfrom
ecstatic-morse:issue-73431

Conversation

@ecstatic-morse

@ecstatic-morseecstatic-morse commented Jun 17, 2020

Copy link
Copy Markdown
Contributor

Fixes (on master) #73431.

Ideally, CustomEq would emit a strict subset of the structural match errors that are found by search_for_structural_match_violation, since it allows more cases due to value-based reasoning. However, const qualification is more conservative than search_for_structural_match_violation around associated constants, since qualification does not try to substitute type parameters.

In the long term, we should probably make const qualification work for generic associated constants, but I don't like extending its capabilities even further.

r? @pnkfelix

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 17, 2020
@ecstatic-morseecstatic-morse added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 17, 2020
@pnkfelix

Copy link
Copy Markdown
Contributor

Should the warning text reference some issue for further info?

@ecstatic-morse

Copy link
Copy Markdown
ContributorAuthor

Done. See #73448.

@pnkfelix

Copy link
Copy Markdown
Contributor

As discussed in private with @ecstatic-morse , i cannot immediately tell if this represents a hole in the structural-match check that we will need to plug (and thus maybe we should have something stronger than a warn! here, which will only fire if one uses RUSTC_LOG=warn or similar).

But it probably isn't worth the effort of putting in the plumbing to issue a proper lint warning here; instead effort should be focused on fixing #73448 itself.

and in any case, this PR as it stands fixes the ICE, which trumps all else at the moment.

@bors r+

@bors

bors commented Jun 17, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 3a1207f has been approved by pnkfelix

@borsbors 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 Jun 17, 2020
@pnkfelix

Copy link
Copy Markdown
Contributor

unilaterally accepting for beta-backport.

@pnkfelixpnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jun 17, 2020
@Dylan-DPC-zz

Dylan-DPC-zz commented Jun 17, 2020

Copy link
Copy Markdown

giving this priority since it is a backport accepted for backport
@bors p=1

tmandry added a commit to tmandry/rust that referenced this pull request Jun 17, 2020
Make novel structural match violations not a `bug`
Fixes (on master) rust-lang#73431.
Ideally, `CustomEq` would emit a strict subset of the structural match errors that are found by `search_for_structural_match_violation`, since it allows more cases due to value-based reasoning. However, const qualification is more conservative than `search_for_structural_match_violation` around associated constants, since qualification does not try to substitute type parameters.
In the long term, we should probably make const qualification work for generic associated constants, but I don't like extending its capabilities even further.
r? @pnkfelix
@tmandrytmandry mentioned this pull request Jun 17, 2020
RalfJung added a commit to RalfJung/rust that referenced this pull request Jun 18, 2020
Make novel structural match violations not a `bug`
Fixes (on master) rust-lang#73431.
Ideally, `CustomEq` would emit a strict subset of the structural match errors that are found by `search_for_structural_match_violation`, since it allows more cases due to value-based reasoning. However, const qualification is more conservative than `search_for_structural_match_violation` around associated constants, since qualification does not try to substitute type parameters.
In the long term, we should probably make const qualification work for generic associated constants, but I don't like extending its capabilities even further.
r? @pnkfelix
@RalfJungRalfJung mentioned this pull request Jun 18, 2020
@bors

bors commented Jun 18, 2020

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 3a1207f with merge 205c00429373b16ac70c645031eabc44fe4c8232...

@RalfJung

Copy link
Copy Markdown
Member

Yielding to rollup
@bors retry

@bors

bors commented Jun 18, 2020

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 3a1207f with merge 493d2a798de149fba00e07f2fb8f0f25f562b760...

@RalfJung

Copy link
Copy Markdown
Member

@bors retry

@bors

bors commented Jun 18, 2020

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 3a1207f with merge b7a9cf51cc76a4992329c385a49862503e968c3e...

@bors

bors commented Jun 18, 2020

Copy link
Copy Markdown
Collaborator

💥 Test timed out

@borsbors 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 Jun 18, 2020
@Dylan-DPC-zz

Copy link
Copy Markdown

@bors retry p=5

@borsbors 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 Jun 18, 2020
@bors

bors commented Jun 18, 2020

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 3a1207f with merge 036b5fe...

@bors

bors commented Jun 19, 2020

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-azure
Approved by: pnkfelix
Pushing 036b5fe to master...

@borsbors added the merged-by-bors This PR was explicitly merged by bors. label Jun 19, 2020
@bors
bors merged commit 036b5fe into rust-lang:masterJun 19, 2020
@Mark-SimulacrumMark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 26, 2020
@Mark-SimulacrumMark-Simulacrum mentioned this pull request Jun 26, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 27, 2020
…ulacrum
[beta] backports
This PR backports the following:
* Make novel structural match violations not a `bug` rust-lang#73446
* linker: Never pass `-no-pie` to non-gnu linkers rust-lang#73384
* Disable the `SimplifyArmIdentity` pass rust-lang#73262
* Allow inference regions when relating consts rust-lang#73225
* Fix link error with #[thread_local] introduced by rust-lang#71192rust-lang#73065
* Ensure stack when building MIR for matches rust-lang#72941
* Don't create impl candidates when obligation contains errors rust-lang#73005
r? @ghost
@ecstatic-morse
ecstatic-morse deleted the issue-73431 branch October 6, 2020 01:42
@cuvipercuviper added this to the 1.46 milestone May 2, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

beta-acceptedAccepted for backporting to the compiler in the beta channel.merged-by-borsThis PR was explicitly merged by bors.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@ecstatic-morse@pnkfelix@bors@Dylan-DPC-zz@RalfJung@cuviper@Mark-Simulacrum@rust-highfive