remove obsolete `givens` from regionck #109629

aliemjay · 2023-03-26T11:47:48Z

Revives #107376. The only change is the last commit (2a3177a) which should fix the regression.

Fixes #106567

r? @lcnr

See rust-lang#109628.

compiler-errors · 2023-03-26T20:04:05Z

@bors try @rust-timer queue

bors · 2023-03-26T20:04:14Z

⌛ Trying commit 2a3177a with merge dfa82e8833199efd2cc0114fed03eab53614a221...

bors · 2023-03-26T22:05:06Z

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

bors · 2023-03-26T22:05:06Z

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

rust-timer · 2023-03-27T01:21:43Z

Finished benchmarking commit (dfa82e8833199efd2cc0114fed03eab53614a221): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

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

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.1%	[1.1%, 1.1%]	1
Regressions ❌ (secondary)	4.0%	[4.0%, 4.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.8%	[-2.8%, -0.9%]	2
All ❌✅ (primary)	1.1%	[1.1%, 1.1%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

lcnr · 2023-03-27T09:05:23Z

hmm, I would prefer to go ahead with landing #109482 instead 🤔

edit: or well, nevermind, i guess it's fine to drop implied bounds for region vars

lcnr · 2023-03-27T09:54:25Z

@bors r+ rollup

bors · 2023-03-27T09:54:28Z

📌 Commit 2a3177a has been approved by lcnr

It is now in the queue for this repository.

lqd · 2023-03-27T09:59:03Z

How about we do a crater run this time, in case they are regressions like last time ?

lcnr · 2023-03-27T10:01:26Z

good catch, yeah 😅 should do

@bors r-

@craterbot check

craterbot · 2023-03-27T10:01:32Z

👌 Experiment pr-109629 created and queued.
🤖 Automatically detected try build dfa82e8833199efd2cc0114fed03eab53614a221
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2023-03-27T10:01:36Z

🚧 Experiment pr-109629 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2023-03-28T08:09:01Z

🎉 Experiment pr-109629 is completed!
📊 1 regressed and 2 fixed (259698 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

aliemjay · 2023-03-28T08:29:23Z

🎉 this is the least noisy crater report I've ever seen.
I think that's enough testament to how good this PR is 😄.

lqd · 2023-03-28T09:36:10Z

It's been a long time since I've seen so little noise as well 😅.

The single xpq "regression" looks like a spurious linking error, and doesn't reproduce locally anyway.

@bors r=lcnr rollup

bors · 2023-03-28T09:36:14Z

📌 Commit 2a3177a has been approved by lcnr

It is now in the queue for this repository.

Rollup of 8 pull requests Successful merges: - rust-lang#91793 (socket ancillary data implementation for FreeBSD (from 13 and above).) - rust-lang#92284 (Change advance(_back)_by to return the remainder instead of the number of processed elements) - rust-lang#102472 (stop special-casing `'static` in evaluation) - rust-lang#108480 (Use Rayon's TLV directly) - rust-lang#109321 (Erase impl regions when checking for impossible to eagerly monomorphize items) - rust-lang#109470 (Correctly substitute GAT's type used in `normalize_param_env` in `check_type_bounds`) - rust-lang#109562 (Update ar_archive_writer to 0.1.3) - rust-lang#109629 (remove obsolete `givens` from regionck) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

compiler-errors · 2023-09-04T17:10:02Z

compiler/rustc_trait_selection/src/traits/outlives_bounds.rs

@@ -52,6 +53,10 @@ impl<'a, 'tcx: 'a> InferCtxtExt<'a, 'tcx> for InferCtxt<'tcx> {
        body_id: LocalDefId,
        ty: Ty<'tcx>,
    ) -> Vec<OutlivesBound<'tcx>> {
+        let ty = self.resolve_vars_if_possible(ty);
+        let ty = OpportunisticRegionResolver::new(self).fold_ty(ty);


Why do we need to resolve region vars here? cc #112832

to avoid any assumptions using inference vars. E.g. if you have 'a = '2 and then get an implied bound '2: 'b, then getting 'a: 'b from that requires us to know that 'a = '2 is something we can assume and not something we have to prove.

All region vars which are part of the tpye before calling implied_outlives_bounds are from candidates used while normalizing the ty, so we can assume that equality holds there.

We need to make sure that we don't use a region bound we have to prove to strengthen an assumption. Had an example for that at some point, would have to look for it though.

aliemjay added 5 commits March 26, 2023 11:43

remove obsolete givens from regionck

b197f1a

resolve regions before implied bounds

d1743c9

add test

9cfb5f7

exhaustive match on implied bounds regions

b20aa97

tolerate region vars in implied bounds

2a3177a

See rust-lang#109628.

rustbot assigned lcnr Mar 26, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 26, 2023

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 26, 2023

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 27, 2023

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 Mar 27, 2023

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 27, 2023

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 27, 2023

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Mar 28, 2023

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 Mar 28, 2023

Noratrieb mentioned this pull request Mar 28, 2023

Rollup of 8 pull requests #109692

Merged

bors merged commit 60ce19d into rust-lang:master Mar 28, 2023

rustbot added this to the 1.70.0 milestone Mar 28, 2023

aliemjay deleted the remove-givens branch April 6, 2023 16:23

Noratrieb mentioned this pull request Jun 20, 2023

ICE: assertion failed: !ty.needs_infer() #112832

Closed

compiler-errors reviewed Sep 4, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove obsolete `givens` from regionck #109629

remove obsolete `givens` from regionck #109629

aliemjay commented Mar 26, 2023 •

edited

Loading

compiler-errors commented Mar 26, 2023

This comment has been minimized.

bors commented Mar 26, 2023

bors commented Mar 26, 2023

bors commented Mar 26, 2023

This comment has been minimized.

rust-timer commented Mar 27, 2023

lcnr commented Mar 27, 2023 •

edited

Loading

lcnr commented Mar 27, 2023

bors commented Mar 27, 2023

lqd commented Mar 27, 2023

lcnr commented Mar 27, 2023

craterbot commented Mar 27, 2023

craterbot commented Mar 27, 2023

craterbot commented Mar 28, 2023

aliemjay commented Mar 28, 2023

lqd commented Mar 28, 2023

bors commented Mar 28, 2023

compiler-errors Sep 4, 2023 •

edited

Loading

lcnr Sep 5, 2023

remove obsolete givens from regionck #109629

remove obsolete givens from regionck #109629

Conversation

aliemjay commented Mar 26, 2023 • edited Loading

compiler-errors commented Mar 26, 2023

This comment has been minimized.

bors commented Mar 26, 2023

bors commented Mar 26, 2023

bors commented Mar 26, 2023

This comment has been minimized.

rust-timer commented Mar 27, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

lcnr commented Mar 27, 2023 • edited Loading

lcnr commented Mar 27, 2023

bors commented Mar 27, 2023

lqd commented Mar 27, 2023

lcnr commented Mar 27, 2023

craterbot commented Mar 27, 2023

craterbot commented Mar 27, 2023

craterbot commented Mar 28, 2023

aliemjay commented Mar 28, 2023

lqd commented Mar 28, 2023

bors commented Mar 28, 2023

compiler-errors Sep 4, 2023 • edited Loading

Choose a reason for hiding this comment

lcnr Sep 5, 2023

Choose a reason for hiding this comment

remove obsolete `givens` from regionck #109629

remove obsolete `givens` from regionck #109629

aliemjay commented Mar 26, 2023 •

edited

Loading

lcnr commented Mar 27, 2023 •

edited

Loading

compiler-errors Sep 4, 2023 •

edited

Loading