Be more conservative about discarding caller_bound in `ParamEnv::and` #84472

Aaron1011 · 2021-04-23T05:56:06Z

No description provided.

rust-highfive · 2021-04-23T05:56:09Z

r? @lcnr

(rust-highfive has picked a reviewer for you, use r? to override)

Aaron1011 · 2021-04-23T05:56:32Z

@bors try @rust-timer queue

rust-timer · 2021-04-23T05:56:33Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2021-04-23T05:56:41Z

⌛ Trying commit 45e1f5e84760d00b9c7fd85bb2e06ec785240825 with merge 9885efde5b04a2021784d1ddfafa8ece66398251...

bors · 2021-04-23T06:57:04Z

☀️ Try build successful - checks-actions
Build commit: 9885efde5b04a2021784d1ddfafa8ece66398251 (9885efde5b04a2021784d1ddfafa8ece66398251)

rust-timer · 2021-04-23T06:57:05Z

Queued 9885efde5b04a2021784d1ddfafa8ece66398251 with parent cb81dc5, future comparison URL.

lcnr

This does seem right to me, is there a specific reason you've made this change? So are there some tests that break without this change?

compiler/rustc_middle/src/ty/flags.rs

lcnr · 2021-04-23T08:33:55Z

compiler/rustc_middle/src/ty/mod.rs

@@ -1207,7 +1207,7 @@ impl<'tcx> ParamEnv<'tcx> {
            Reveal::UserFacing => ParamEnvAnd { param_env: self, value },

            Reveal::All => {
-                if value.is_global() {
+                if value.is_global() && !value.has_ty_fresh() {


hmm, i would assume that has_ty_fresh should instead be a subset of is_global. i.e. i would add HAS_TY_FRESH and HAS_CT_FRESH to HAS_FREE_LOCAL_NAMES 🤔

After some further thought, I think you're correct. I originally decided against doing that because I was worried about affecting unrelated calls of is_global(). However, it should always be correct to return false from is_global, since is_global() == true is used for caching and bypassing checks.

compiler/rustc_type_ir/src/lib.rs

rust-timer · 2021-04-23T11:08:41Z

Finished benchmarking try commit (9885efde5b04a2021784d1ddfafa8ece66398251): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

Aaron1011 · 2021-04-23T18:13:07Z

I meant to mark this as WIP - it's a fix for #84399, but it still needs some cleanup and comments. I opened it for a perf run

This ensures that `ParamEnv::and` preserves the original `caller_bounds` when we have a value containing fresh tys/consts. This ensures that when we cache a `SelectionCandidate`, the cache key (a `ParamEnvAnd`) contains all of the information that influenced the computation of our result (e.g. we may end up choosing a `ParamCandidate`)

Aaron1011 · 2021-05-01T20:59:27Z

@bors try @rust-timer queue

rust-timer · 2021-05-01T20:59:29Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2021-05-01T20:59:36Z

⌛ Trying commit 91daf70 with merge d6198ca66b6828db3167ab78e7298795c0140b93...

bors · 2021-05-01T21:47:30Z

☀️ Try build successful - checks-actions
Build commit: d6198ca66b6828db3167ab78e7298795c0140b93 (d6198ca66b6828db3167ab78e7298795c0140b93)

rust-timer · 2021-05-01T21:47:32Z

Queued d6198ca66b6828db3167ab78e7298795c0140b93 with parent 6e2a344, future comparison URL.

rust-timer · 2021-05-02T00:00:07Z

Finished benchmarking try commit (d6198ca66b6828db3167ab78e7298795c0140b93): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

bors · 2021-05-03T10:38:43Z

⌛ Testing commit 091b7dd with merge e80ce7a23452e0a0be82cf218b4c64ca6003870e...

bors · 2021-05-03T10:44:03Z

💔 Test failed - checks-actions

rust-log-analyzer · 2021-05-03T10:52:24Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

 37  516M   37  194M    0     0  26.5M      0  0:00:19  0:00:07  0:00:12 26.8M
 39  516M   39  201M    0     0  26.5M      0  0:00:19  0:00:07  0:00:12 27.0M
curl: (56) OpenSSL SSL_read: Connection was reset, errno 10054

gzip: stdin: unexpected end of file
tar: Unexpected EOF in archive
tar: Unexpected EOF in archive
tar: Error is not recoverable: exiting now
##[error]Process completed with exit code 2.
[command]"C:\Program Files\Git\bin\git.exe" version
git version 2.31.1.windows.1
[command]"C:\Program Files\Git\bin\git.exe" config --local --name-only --get-regexp core\.sshCommand
[command]"C:\Program Files\Git\bin\git.exe" submodule foreach --recursive "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
[command]"C:\Program Files\Git\bin\git.exe" submodule foreach --recursive "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
[command]"C:\Program Files\Git\bin\git.exe" config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
http.https://github.com/.extraheader
[command]"C:\Program Files\Git\bin\git.exe" config --local --unset-all http.https://github.com/.extraheader
[command]"C:\Program Files\Git\bin\git.exe" submodule foreach --recursive "git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :"
Cleaning up orphan processes
Terminate orphan process: pid (8080) (node)
Terminate orphan process: pid (4700) (python)
Terminate orphan process: pid (5592) (vctip)

Aaron1011 · 2021-05-03T12:42:58Z

@bors retry

bors · 2021-05-03T17:00:21Z

⌛ Testing commit 091b7dd with merge dc0040a97af0adecdff4fd1b44e717be81110a11...

bors · 2021-05-03T18:51:46Z

💔 Test failed - checks-actions

rust-log-analyzer · 2021-05-03T18:51:49Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

Aaron1011 · 2021-05-03T19:44:38Z

@bors retry

bors · 2021-05-03T20:02:19Z

⌛ Testing commit 091b7dd with merge b5588c22d430f067d0db3d8cdad961e8341da960...

Mark-Simulacrum · 2021-05-03T20:06:02Z

@bors retry - yield to stable

rust-log-analyzer · 2021-05-03T20:13:34Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

Mark-Simulacrum · 2021-05-04T00:48:38Z

@bors treeclosed-

bors · 2021-05-04T00:48:46Z

⌛ Testing commit 091b7dd with merge 14f863c...

bors · 2021-05-04T03:14:27Z

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing 14f863c to master...

rust-highfive assigned lcnr Apr 23, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 23, 2021

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 23, 2021

lcnr reviewed Apr 23, 2021

View reviewed changes

klensy reviewed Apr 23, 2021

View reviewed changes

compiler/rustc_type_ir/src/lib.rs Show resolved Hide resolved

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 23, 2021

Aaron1011 changed the title ~~Be more conservative about discarding caller_bound in ParamEnv::and~~ [WIP] Be more conservative about discarding caller_bound in ParamEnv::and Apr 23, 2021

Aaron1011 mentioned this pull request Apr 23, 2021

ICE when compiling some specific set of traits defined for nalgebra's matrix #84399

Closed

Aaron1011 force-pushed the conservative-paramenv-and branch from 45e1f5e to 91daf70 Compare May 1, 2021 20:59

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 1, 2021

Add regression test

091b7dd

Aaron1011 changed the title ~~[WIP] Be more conservative about discarding caller_bound in ParamEnv::and~~ Be more conservative about discarding caller_bound in ParamEnv::and May 1, 2021

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 2, 2021

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label May 3, 2021

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 May 3, 2021

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 May 3, 2021

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 May 3, 2021

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 May 3, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label May 4, 2021

bors merged commit 14f863c into rust-lang:master May 4, 2021

rustbot added this to the 1.54.0 milestone May 4, 2021

Alexendoo mentioned this pull request May 5, 2021

ICE: feature(test), requires two tests in lib #78632

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Be more conservative about discarding caller_bound in `ParamEnv::and` #84472

Be more conservative about discarding caller_bound in `ParamEnv::and` #84472

Aaron1011 commented Apr 23, 2021

rust-highfive commented Apr 23, 2021

Aaron1011 commented Apr 23, 2021

rust-timer commented Apr 23, 2021

bors commented Apr 23, 2021

bors commented Apr 23, 2021

rust-timer commented Apr 23, 2021

lcnr left a comment

lcnr Apr 23, 2021

Aaron1011 May 1, 2021

rust-timer commented Apr 23, 2021

Aaron1011 commented Apr 23, 2021 •

edited

Loading

Aaron1011 commented May 1, 2021

rust-timer commented May 1, 2021

bors commented May 1, 2021

bors commented May 1, 2021

rust-timer commented May 1, 2021

rust-timer commented May 2, 2021

bors commented May 3, 2021

bors commented May 3, 2021

rust-log-analyzer commented May 3, 2021

Aaron1011 commented May 3, 2021

bors commented May 3, 2021

bors commented May 3, 2021

rust-log-analyzer commented May 3, 2021

Aaron1011 commented May 3, 2021

bors commented May 3, 2021

Mark-Simulacrum commented May 3, 2021

rust-log-analyzer commented May 3, 2021

Mark-Simulacrum commented May 4, 2021

bors commented May 4, 2021

bors commented May 4, 2021

Be more conservative about discarding caller_bound in ParamEnv::and #84472

Be more conservative about discarding caller_bound in ParamEnv::and #84472

Conversation

Aaron1011 commented Apr 23, 2021

rust-highfive commented Apr 23, 2021

Aaron1011 commented Apr 23, 2021

rust-timer commented Apr 23, 2021

bors commented Apr 23, 2021

bors commented Apr 23, 2021

rust-timer commented Apr 23, 2021

lcnr left a comment

Choose a reason for hiding this comment

lcnr Apr 23, 2021

Choose a reason for hiding this comment

Aaron1011 May 1, 2021

Choose a reason for hiding this comment

rust-timer commented Apr 23, 2021

Aaron1011 commented Apr 23, 2021 • edited Loading

Aaron1011 commented May 1, 2021

rust-timer commented May 1, 2021

bors commented May 1, 2021

bors commented May 1, 2021

rust-timer commented May 1, 2021

rust-timer commented May 2, 2021

bors commented May 3, 2021

bors commented May 3, 2021

rust-log-analyzer commented May 3, 2021

Aaron1011 commented May 3, 2021

bors commented May 3, 2021

bors commented May 3, 2021

rust-log-analyzer commented May 3, 2021

Aaron1011 commented May 3, 2021

bors commented May 3, 2021

Mark-Simulacrum commented May 3, 2021

rust-log-analyzer commented May 3, 2021

Mark-Simulacrum commented May 4, 2021

bors commented May 4, 2021

bors commented May 4, 2021

Be more conservative about discarding caller_bound in `ParamEnv::and` #84472

Be more conservative about discarding caller_bound in `ParamEnv::and` #84472

Aaron1011 commented Apr 23, 2021 •

edited

Loading