Aggressively check for non-`PartialEq` types in `const_to_pat` #72184

ecstatic-morse · 2020-05-14T03:15:17Z

Resolves #65466.

Previously, we allowed constants without structural match violations through to codegen even if they did not satisfy PartialEq. This was because higher-ranked types such as for<'r> fn(&'r i32) needed to be allowed in patterns, despite the fact that they cannot be proven to be PartialEq. I think this is due to a limitation in the trait solver?

This PR falls back to a terrible hack–replacing all late-bound regions with concrete ones–after initially failing to prove PartialEq. This succeeds for higher-ranked types that would implement PartialEq with some unconstrained set of concrete lifetimes, such as the aforementioned function pointer. Now we can reject types that don't satisfy PartialEq without breaking backwards compatibility for function pointers. This prevents some ICEs during codegen.

cc rust-lang/rfcs#1445

r? @pnkfelix (diff -w recommended)

bors · 2020-05-14T23:26:51Z

☔ The latest upstream changes (presumably #72202) made this pull request unmergeable. Please resolve the merge conflicts.

rust-highfive · 2020-05-17T00:25:26Z

The job mingw-check of your PR failed (pretty log, 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.

##[section]Starting: Linux mingw-check
##[section]Starting: Initialize job
Agent name: 'Azure Pipelines 37'
Agent machine name: 'fv-az659'
Current agent version: '2.168.2'
##[group]Operating System
16.04.6
LTS
LTS
##[endgroup]
##[group]Virtual Environment
Environment: ubuntu-16.04
Version: 20200430.2
Included Software: https://github.com/actions/virtual-environments/blob/ubuntu16/20200430.2/images/linux/Ubuntu1604-README.md
##[endgroup]
Agent running as: 'vsts'
Prepare build directory.
Set build variables.
Download all required tasks.
Download all required tasks.
Downloading task: Bash (3.163.2)
Checking job knob settings.
   Knob: AgentToolsDirectory = /opt/hostedtoolcache Source: ${AGENT_TOOLSDIRECTORY} 
   Knob: AgentPerflog = /home/vsts/perflog Source: ${VSTS_AGENT_PERFLOG} 
Start tracking orphan processes.
##[section]Finishing: Initialize job
##[section]Starting: Configure Job Name
==============================================================================
---
========================== Starting Command Output ===========================
[command]/bin/bash --noprofile --norc /home/vsts/work/_temp/b21b32e0-f6f0-448c-8e55-f90b6221c8a8.sh

##[section]Finishing: Disable git automatic line ending conversion
##[section]Starting: Checkout rust-lang/rust@refs/pull/72184/merge to s
Task         : Get sources
Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
Version      : 1.0.0
Author       : Microsoft
---
##[command]git remote add origin https://github.com/rust-lang/rust
##[command]git config gc.auto 0
##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
##[command]git config --get-all http.proxy
##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/72184/merge:refs/remotes/pull/72184/merge
---
 ---> 3adb0605cc65
Step 6/7 : ENV RUN_CHECK_WITH_PARALLEL_QUERIES 1
 ---> Using cache
 ---> 28dbc326cb7f
Step 7/7 : ENV SCRIPT python3 ../x.py test src/tools/expand-yaml-anchors &&            python3 ../x.py check --target=i686-pc-windows-gnu --host=i686-pc-windows-gnu &&            python3 ../x.py build --stage 0 src/tools/build-manifest &&            python3 ../x.py test --stage 0 src/tools/compiletest &&            python3 ../x.py test src/tools/tidy &&            python3 ../x.py doc --stage 0 src/libstd &&            /scripts/validate-toolstate.sh
 ---> 537a01811900
Successfully built 537a01811900
Successfully tagged rust-ci:latest
Built container sha256:537a018119009dc218456238dec90b5530050db1e2a1e166550c218003f6159d
---
    Checking rustc_feature v0.0.0 (/checkout/src/librustc_feature)
    Checking fmt_macros v0.0.0 (/checkout/src/libfmt_macros)
    Checking rustc_ast_pretty v0.0.0 (/checkout/src/librustc_ast_pretty)
    Checking rustc_hir v0.0.0 (/checkout/src/librustc_hir)
    Checking rustc_query_system v0.0.0 (/checkout/src/librustc_query_system)
    Checking chalk-rust-ir v0.10.0
    Checking rustc_parse v0.0.0 (/checkout/src/librustc_parse)
    Checking rustc_hir_pretty v0.0.0 (/checkout/src/librustc_hir_pretty)
    Checking rustc_ast_lowering v0.0.0 (/checkout/src/librustc_ast_lowering)
---
    Checking rustc_lint v0.0.0 (/checkout/src/librustc_lint)
    Checking rustc_passes v0.0.0 (/checkout/src/librustc_passes)
    Checking rustc_mir_build v0.0.0 (/checkout/src/librustc_mir_build)
    Checking rustc_mir v0.0.0 (/checkout/src/librustc_mir)
error: unused import: `rustc_hir::lang_items::EqTraitLangItem`
  |
  |
2 | use rustc_hir::lang_items::EqTraitLangItem;
  |
  = note: `-D unused-imports` implied by `-D warnings`

error: aborting due to previous error
error: aborting due to previous error

error: could not compile `rustc_mir_build`.

To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error: build failed
command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "check" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "2" "--release" "--color" "always" "--features" " llvm" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "--message-format" "json-render-diagnostics"
failed to run: /checkout/obj/build/bootstrap/debug/bootstrap check
Build completed unsuccessfully in 0:04:52
== clock drift check ==
  local time: Sun May 17 00:11:42 UTC 2020
  local time: Sun May 17 00:11:42 UTC 2020
  network time: Sun, 17 May 2020 00:11:42 GMT
== end clock drift check ==

##[error]Bash exited with code '1'.
##[section]Finishing: Run build
##[section]Starting: Checkout rust-lang/rust@refs/pull/72184/merge to s
Task         : Get sources
Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
Version      : 1.0.0
Author       : Microsoft
Author       : Microsoft
Help         : [More Information](https://go.microsoft.com/fwlink/?LinkId=798199)
==============================================================================
Cleaning any cached credential from repository: rust-lang/rust (GitHub)
##[section]Finishing: Checkout rust-lang/rust@refs/pull/72184/merge to s
Cleaning up task key
Start cleaning up orphan processes.
Terminate orphan process: pid (3452) (python)
##[section]Finishing: Finalize Job

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 @rust-lang/infra. (Feature Requests)

pnkfelix · 2020-05-20T02:57:18Z

src/librustc_mir_build/hair/pattern/const_to_pat.rs

+        // ones. This leverages the fact that function pointers with no late-bound lifetimes do
+        // satisfy `PartialEq`. In other words, we transform `Option<for<'r> fn(&'r i32)>` to
+        // `Option<fn(&'erased i32)>` and again check whether `PartialEq` is satisfied.
+        // Obviously this is too permissive, but it is better than the old behavior, which


It would probably be a good idea to construct an example of something that runs into this (and presumably causes the same ICE that this is fixing most other occurrences of), and then file a bug about fixing that remaining hole, right?

@ecstatic-morse , can you provide a piece of example code that runs into that scenario?

My thought was that you could write something like impl PartialEq for &'static MyStruct and then somehow write a higher-ranked reference to MyStruct in a const. Depending on how the trait solver treats 'erased, that might still cause an ICE.

However, I think the only way we let the user write higher-ranked types is with fn(&i32) or Fn(&i32), neither of which can run into that problem. Additionally, users can no longer use trait objects in patterns since #71038. If these really are all the ways that the user can write higher-ranked types, perhaps this exactly the right amount of permissive?

pnkfelix · 2020-05-20T02:59:20Z

This seems fine, as long as I'm correct in my understanding that this is just turning some ICE's from #65466 (most, to be fair) into static errors, while leaving the remaining ICEs from #65466 as ICEs.

That is, the note about "this is too permissive, obviously" -- that doesn't correspond to any case where we previously had an ICE (or static error) and would now attempt to do codegen for invalid code, right?

bors · 2020-05-25T21:07:21Z

☔ The latest upstream changes (presumably #72575) made this pull request unmergeable. Please resolve the merge conflicts.

ecstatic-morse · 2020-06-11T20:56:08Z

@pnkfelix I'm just going to close this PR since #72184 (comment) has gone unanswered for so long. It was prompted by #67343 (comment). I guess you were right 😄.

rust-highfive assigned pnkfelix May 14, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 14, 2020

ecstatic-morse force-pushed the issue-65466 branch from 9009d34 to 78e3312 Compare May 17, 2020 00:02

ecstatic-morse added 3 commits May 16, 2020 19:28

Check all types for PartialEq while lowering consts to patterns

e390944

Bless new diagnostics

ca53cda

Run regression test for rust-lang#65466

2137e0e

ecstatic-morse force-pushed the issue-65466 branch from 78e3312 to 2137e0e Compare May 17, 2020 02:28

pnkfelix reviewed May 20, 2020

View reviewed changes

pnkfelix added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 20, 2020

Elinvynia added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 3, 2020

ecstatic-morse closed this Jun 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Aggressively check for non-`PartialEq` types in `const_to_pat` #72184

Aggressively check for non-`PartialEq` types in `const_to_pat` #72184

ecstatic-morse commented May 14, 2020 •

edited

Loading

bors commented May 14, 2020

rust-highfive commented May 17, 2020

pnkfelix May 20, 2020

ecstatic-morse May 20, 2020 •

edited

Loading

pnkfelix commented May 20, 2020

bors commented May 25, 2020

ecstatic-morse commented Jun 11, 2020

Aggressively check for non-PartialEq types in const_to_pat #72184

Aggressively check for non-PartialEq types in const_to_pat #72184

Conversation

ecstatic-morse commented May 14, 2020 • edited Loading

bors commented May 14, 2020

rust-highfive commented May 17, 2020

pnkfelix May 20, 2020

Choose a reason for hiding this comment

ecstatic-morse May 20, 2020 • edited Loading

Choose a reason for hiding this comment

pnkfelix commented May 20, 2020

bors commented May 25, 2020

ecstatic-morse commented Jun 11, 2020

Aggressively check for non-`PartialEq` types in `const_to_pat` #72184

Aggressively check for non-`PartialEq` types in `const_to_pat` #72184

ecstatic-morse commented May 14, 2020 •

edited

Loading

ecstatic-morse May 20, 2020 •

edited

Loading