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

Separate AnonConst from ConstBlock in HIR. #109609

Merged
merged 1 commit into from
Jun 3, 2023
Merged

Separate AnonConst from ConstBlock in HIR. #109609

merged 1 commit into from
Jun 3, 2023

Conversation

cjgillot
Copy link
Contributor

Their behaviours are different enough to justify having separate nodes.

@rustbot
Copy link
Collaborator

rustbot commented Mar 25, 2023

r? @lcnr

(rustbot has picked a reviewer for you, use r? to override)

@rustbot 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 25, 2023
@rustbot
Copy link
Collaborator

rustbot commented Mar 25, 2023

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

compiler-errors
compiler-errors reviewed Mar 26, 2023
View reviewed changes
compiler/rustc_passes/src/dead.rs
@@ -465,6 +465,17 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {

self.in_pat = in_pat;
}

fn visit_inline_const(&mut self, c: &'tcx hir::ConstBlock) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need the above visit_anon_const if const blocks should be handled in visit_inline_const instead?

Copy link
Contributor Author

@cjgillot cjgillot Apr 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. You still want to mark three as live in:

match S::<3> {
    S::<{three()}> => {}
}

@BoxyUwU
Copy link
Member

BoxyUwU commented Mar 30, 2023

r? @BoxyUwU

@rustbot rustbot assigned BoxyUwU and unassigned lcnr Mar 30, 2023
@bors
Copy link
Contributor

bors commented Apr 21, 2023

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

@BoxyUwU
Copy link
Member

BoxyUwU commented Apr 27, 2023 •
edited
Loading

What are the differing behaviours of anon consts and inline const that motivate this? Off the top of my head inline consts get typeck'd with their parent item and have weirder generics/typeof to allow for that wheras other anon consts get typeckd by themselves 🤔

@bors
Copy link
Contributor

bors commented May 9, 2023

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

@apiraino
Copy link
Contributor

switching to author to reply to previous comment and rebase - thanks :)

@rustbot author

@rustbot rustbot 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 17, 2023
@cjgillot cjgillot force-pushed the split-anon-const branch from 214cc17 to c11147b Compare May 24, 2023 17:48
@rustbot
Copy link
Collaborator

rustbot commented May 24, 2023

Some changes might have occurred in exhaustiveness checking

cc @Nadrieril

@cjgillot
Copy link
Contributor Author

What are the differing behaviours of anon consts and inline const that motivate this? Off the top of my head inline consts get typeck'd with their parent item and have weirder generics/typeof to allow for that wheras other anon consts get typeckd by themselves thinking

Yes, the difference in typeck root is the main reason to separate those cases IMO. Most of the cases where we end-up with code duplication are in side passes, with much less complexity than type collection.

@cjgillot cjgillot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 24, 2023
BoxyUwU
BoxyUwU approved these changes May 31, 2023
View reviewed changes
Copy link
Member

@BoxyUwU BoxyUwU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for taking so long to get to looking at this properly. r=me after rebasing

compiler/rustc_metadata/src/rmeta/encoder.rs Show resolved Hide resolved
@BoxyUwU BoxyUwU 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 31, 2023
@cjgillot cjgillot force-pushed the split-anon-const branch from c11147b to ca4d0d4 Compare June 2, 2023 21:26
@BoxyUwU
Copy link
Member

BoxyUwU commented Jun 2, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Jun 2, 2023

📌 Commit ca4d0d4 has been approved by BoxyUwU

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 2, 2023
@compiler-errors compiler-errors mentioned this pull request Jun 2, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 2, 2023
@bors
Auto merge of rust-lang#112228 - compiler-errors:rollup-97i0pli, r=co…
7a0070e 
…mpiler-errors

Rollup of 6 pull requests

Successful merges:

 - rust-lang#109609 (Separate AnonConst from ConstBlock in HIR.)
 - rust-lang#112166 (bootstrap: Rename profile = user to profile = dist)
 - rust-lang#112168 (Lower `unchecked_div`/`_rem` to MIR's `BinOp::Div`/`Rem`)
 - rust-lang#112183 (Normalize anon consts in new solver)
 - rust-lang#112211 (pass `--lib` to `x doc`)
 - rust-lang#112223 (Don't ICE in new solver when auto traits have associated types)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6c9b7d6 into rust-lang:master Jun 3, 2023
@rustbot rustbot added this to the 1.72.0 milestone Jun 3, 2023
@cjgillot cjgillot deleted the split-anon-const branch June 3, 2023 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compiler-errors compiler-errors compiler-errors left review comments

@BoxyUwU BoxyUwU BoxyUwU approved these changes

Assignees

@BoxyUwU BoxyUwU

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.72.0
Development

Successfully merging this pull request may close these issues.
7 participants
@cjgillot @rustbot @BoxyUwU @bors @apiraino @compiler-errors @lcnr