Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

move const param structural match checks to wfcheck #75069

Merged
merged 3 commits into from
Aug 19, 2020
Merged

move const param structural match checks to wfcheck #75069

merged 3 commits into from
Aug 19, 2020

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Aug 2, 2020 •
edited
Loading

fixes #75047 fixes #74950

We currently check for structural match violations inside of type_of.
As we need to check the array length when checking if [NonEq; arr_len] is structural match, we potentially require the variance of an expression. Computing the variance requires type_of for all types though, resulting in a cycle error.

r? @varkor @eddyb

@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 2, 2020
@lcnr
Copy link
Contributor Author

lcnr commented Aug 2, 2020

This PR fixes this by moving structural match checks to wfcheck which is now potentially slower, so this requires a perf run.

@bors rollup=never

@lcnr
Copy link
Contributor Author

lcnr commented Aug 2, 2020

Apparently this removes some additional overflow errors 🤔 not sure what's going on there,
probably have to look a bit more into why this happens before we can merge this

@bors
Copy link
Contributor

bors commented Aug 8, 2020

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

@lcnr
Copy link
Contributor Author

lcnr commented Aug 12, 2020

I guess this is now ready for review.

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Aug 12, 2020

⌛ Trying commit 38a2409e8b5a84b0533eaa1740322457528825a3 with merge 5a6037833343e7527da4d9a756733f67217c3690...

@Aaron1011
Copy link
Member

This also changes wfcheck to not run in parallel anymore, which means that we only emit at most 1 overflow error.

@lcnr Why can wfcheck no longer run in parallel?

@bors
Copy link
Contributor

bors commented Aug 13, 2020

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

@lcnr
Copy link
Contributor Author

lcnr commented Aug 13, 2020 •
edited
Loading

Because par visitor only visits items and Visitor takes &mut self. Just realised how to fix this 😆 let me quickly do so...

@lcnr lcnr force-pushed the type-of-lazy-norm branch 2 times, most recently from b0ea642 to 6da8707 Compare August 14, 2020 07:47
@lcnr
Copy link
Contributor Author

lcnr commented Aug 14, 2020 •
edited
Loading

Okay, wfcheck is once again running in parallel (or at least it pretends to do so 😆 )

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Aug 14, 2020

⌛ Trying commit 6da8707cad88d2956f74f4fcb8ac9d448d5edc89 with merge c8d1fc370f403c259cdb0bafec3becd3762076ff...

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Aug 14, 2020

⌛ Trying commit 6da8707cad88d2956f74f4fcb8ac9d448d5edc89 with merge a2cdf852b7b3bb45fc55553dff1f0bb500b319fb...

@bors
Copy link
Contributor

bors commented Aug 14, 2020

☀️ Try build successful - checks-actions, checks-azure
Build commit: a2cdf852b7b3bb45fc55553dff1f0bb500b319fb (a2cdf852b7b3bb45fc55553dff1f0bb500b319fb)

@rust-timer
Copy link
Collaborator

Queued a2cdf852b7b3bb45fc55553dff1f0bb500b319fb with parent 8e5a277, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (a2cdf852b7b3bb45fc55553dff1f0bb500b319fb): 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

@lcnr
Copy link
Contributor Author

lcnr commented Aug 14, 2020

perf looks neutral to me

@bors rollup-

@varkor
Copy link
Member

varkor commented Aug 14, 2020

I'll try to review soon; sorry for the delay.

@bors
Copy link
Contributor

bors commented Aug 18, 2020

📌 Commit 7542615 has been approved by varkor

@bors 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 Aug 18, 2020
@bors
Copy link
Contributor

bors commented Aug 19, 2020

⌛ Testing commit 7542615 with merge 21c031407e7a52efbadeee3003da606a35733ce9...

@rust-log-analyzer
Copy link
Collaborator

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.

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)

@lcnr
Copy link
Contributor Author

lcnr commented Aug 19, 2020

2020-08-19T09:12:24.5372520Z curl: (56) SSLRead() return error -9806
2020-08-19T09:12:24.5373420Z clang+llvm-9.0.0-x86_64-darwin-apple/bin/clang-query: Lzma library error: No progress is possible
2020-08-19T09:12:24.5473760Z tar: Error exit delayed from previous errors.

@rust-lang/infra is that a spurious error?

@kennytm
Copy link
Member

kennytm commented Aug 19, 2020

yes, but that builder is "fallible", so we could wait for other builders to complete.

@bors
Copy link
Contributor

bors commented Aug 19, 2020

💔 Test failed - checks-azure

@bors 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 Aug 19, 2020
@lcnr
Copy link
Contributor Author

lcnr commented Aug 19, 2020

warning: error: Connection to server timed out

That also seems spurious...

@bors retry

@bors 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 Aug 19, 2020
@bors
Copy link
Contributor

bors commented Aug 19, 2020

⌛ Testing commit 7542615 with merge 1239664923db3134b29287a9cc04c539d9e9b3a8...

tmandry added a commit to tmandry/rust that referenced this pull request Aug 19, 2020
@tmandry
Rollup merge of rust-lang#75069 - lcnr:type-of-lazy-norm, r=varkor
f0b7901 
move const param structural match checks to wfcheck

fixes rust-lang#75047 fixes rust-lang#74950

We currently check for structural match violations inside of `type_of`.
As we need to check the array length when checking if `[NonEq; arr_len]` is structural match, we potentially require the variance of an expression. Computing the variance requires `type_of` for all types though, resulting in a cycle error.

r? @varkor @eddyb
@tmandry tmandry mentioned this pull request Aug 19, 2020
Closed
@tmandry
Copy link
Member

tmandry commented Aug 19, 2020

Rolled up
@bors retry

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Aug 19, 2020

⌛ Testing commit 7542615 with merge 04a457ef6ae72e24e140fc410a099ec582ad32e1...

@tmandry tmandry mentioned this pull request Aug 19, 2020
Merged
@tmandry
Copy link
Member

tmandry commented Aug 19, 2020

@bors retry

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 19, 2020
@bors
Auto merge of rust-lang#75715 - tmandry:rollup-18atkj4, r=tmandry
9900178 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#75069 (move const param structural match checks to wfcheck)
 - rust-lang#75587 (mir building: fix some comments)
 - rust-lang#75593 (Adjust installation place for compiler docs)
 - rust-lang#75648 (Make OnceCell<T> transparent to dropck)
 - rust-lang#75649 (Fix intra-doc links for inherent impls that are both lang items and not the default impl)
 - rust-lang#75674 (Move to intra doc links for std::io)
 - rust-lang#75696 (Remove `#[cfg(miri)]` from OnceCell tests)

Failed merges:

r? @ghost
@bors bors merged commit 672d009 into rust-lang:master Aug 19, 2020
@lcnr lcnr deleted the type-of-lazy-norm branch August 19, 2020 20:23
@lcnr lcnr mentioned this pull request Nov 17, 2020
Closed
16 tasks
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@varkor varkor varkor left review comments

Assignees

@varkor varkor

Labels
F-const_generics `#![feature(const_generics)]` S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.47.0
10 participants
@lcnr @rust-highfive @bors @rust-timer @Aaron1011 @varkor @rust-log-analyzer @kennytm @tmandry @cuviper