4

Find the right lower bound region in the scenario of partial order relations by...

 1 year ago
source link: https://github.com/rust-lang/rust/pull/104765
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client

Conversation

Contributor

@chenyukang chenyukang commented Nov 23, 2022 •

edited

Fixes #104639

aliemjay and faptc reacted with heart emoji

Collaborator

rustbot commented Nov 23, 2022

Failed to set assignee to aliemjay: invalid assignee

Note: Only org members, users with write permissions, or people who have commented on the PR may be assigned.

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

Nov 23, 2022

Contributor

Author

chenyukang commented Nov 23, 2022

Contributor

aliemjay commented Nov 23, 2022

Contributor

@aliemjay aliemjay left a comment

Oops, I missed that.

}

let Some((_, &min_choice)) = choice_regions.iter().enumerate().find(|(i, &r1)| {

choice_regions.iter().enumerate().all(|(j, &r2)| {

*i == j || self.universal_region_relations.outlives(r1, r2)

Contributor

@aliemjay aliemjay Nov 23, 2022

Suggested change
*i == j || self.universal_region_relations.outlives(r1, r2)
self.universal_region_relations.outlives(r2, r1)

The min_choice should be outlived by all others, not outlives all others. I'll be worried if we don't have tests that tripped for this.

There's also no need to check i == j because universal_region_relations is reflexive, so outlives(r1, r1) == true.

Contributor

Author

@chenyukang chenyukang Nov 23, 2022

I just found a local test failed for my change:

async fn async_ret_impl_trait2<'a, 'b>(a: &'a u8, b: &'b u8) -> impl Trait<'a>
where
    'b: 'a,
{
    (a, b)
}

Will fix it.

aliemjay reacted with thumbs up emoji

This comment has been minimized.

Contributor

oli-obk commented Nov 23, 2022

This comment has been minimized.

rustbot

added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label

Nov 23, 2022

Contributor

bors commented Nov 23, 2022

hourglass Trying commit 1fa6738 with merge 0993c8a...

Contributor

oli-obk commented Nov 29, 2022

This comment has been minimized.

Contributor

bors commented Nov 29, 2022

hourglass Trying commit b0d39c6 with merge f9109e9...

Contributor

bors commented Nov 29, 2022

sunny Try build successful - checks-actions
Build commit: f9109e9 (f9109e9fb8f2d10b5ce3c3109963c05723d10c06)

This comment has been minimized.

Collaborator

rust-timer commented Nov 29, 2022

Finished benchmarking commit (f9109e9): comparison URL.

Overall result: xwhite_check_mark regressions and improvements - 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.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions x
(primary)		 0.6% [0.4%, 0.9%] 7
Regressions x
(secondary)		 - - 0
Improvements white_check_mark
(primary)		 -0.9% [-0.9%, -0.8%] 2
Improvements white_check_mark
(secondary)		 -2.1% [-2.7%, -0.3%] 7
All xwhite_check_mark (primary) 0.3% [-0.9%, 0.9%] 9

Max RSS (memory usage)

Results

Cycles

Results

rustbot

added perf-regression Performance regressions

and removed S-waiting-on-perf Status: Waiting on a perf run to be completed.

labels

Nov 29, 2022

Member

lqd commented Nov 30, 2022

The regressions above are likely current noise on these benchmarks -- and the O(N²) change here is probably fine at the low Ns we're expecting to see here ?

oli-obk

added T-types Relevant to the types team, which will review and decide on the PR/issue.

and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

labels

Nov 30, 2022

rfcbot

added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised.

and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off.

labels

Nov 30, 2022

rfcbot commented Nov 30, 2022

bellThis is now entering its final comment period, as per the review above. bell

This comment has been minimized.

rustbot

added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label

Nov 30, 2022

Contributor

bors commented Nov 30, 2022

hourglass Trying commit b0d39c6 with merge 97288fb...

Wait I didn't notice there was a perf run already oops

Contributor

bors commented Dec 1, 2022

sunny Try build successful - checks-actions
Build commit: 97288fb (97288fb87403b68dc2859cabca1c69afad7a3a89)

This comment has been minimized.

Collaborator

rust-timer commented Dec 1, 2022

Finished benchmarking commit (97288fb): 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

Cycles

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

rustbot

removed S-waiting-on-perf Status: Waiting on a perf run to be completed. perf-regression Performance regressions

labels

Dec 1, 2022

rfcbot

added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting

and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised.

labels

Dec 10, 2022

rfcbot commented Dec 10, 2022

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

Contributor

oli-obk commented Dec 12, 2022

@bors r+ rollup=never (for bisectability)

bors never seemed to register this

@bors r=oli-obk rollup=never

Contributor

bors commented Dec 14, 2022

pushpin Commit b0d39c6 has been approved by oli-obk

It is now in the queue for this repository.

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

Dec 14, 2022

Contributor

bors commented Dec 15, 2022

hourglass Testing commit b0d39c6 with merge 939a3dd...

Contributor

bors commented Dec 15, 2022

sunny Test successful - checks-actions
Approved by: oli-obk
Pushing 939a3dd to master...

bors

added the merged-by-bors This PR was explicitly merged by bors label

Dec 15, 2022

bors

merged commit 939a3dd into

rust-lang:master

Dec 15, 2022

11 checks passed

Collaborator

rust-timer commented Dec 15, 2022

Finished benchmarking commit (939a3dd): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

Results

Cycles

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

aliemjay

Assignees

oli-obk

Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. 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-types Relevant to the types team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting
Projects

None yet

Milestone

1.68.0

Development

Successfully merging this pull request may close these issues.

lifetime order breaks member constraints

11 participants

report

Recommend

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK