Skip to content

Refactor branch Clause:: #107603

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.

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

Closed
wants to merge 1 commit into from

Conversation

sladyn98
Copy link
Contributor

@sladyn98 sladyn98 commented Feb 2, 2023

This PR refactors Clause:: to clause::
reexport Clause's variants from the clause module and change all uses of Clause::Foo to clause::Foo and import the clause module where necessary.

Closes: #105060

@rustbot
Copy link
Collaborator

rustbot commented Feb 2, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @estebank (or someone else) soon.

Please see the contribution instructions for more information.

@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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Feb 2, 2023
@rustbot
Copy link
Collaborator

rustbot commented Feb 2, 2023

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

@sladyn98
Copy link
Contributor Author

sladyn98 commented Feb 2, 2023

@spastorino @oli-obk I opened a PR with the first part of 2 parts: I was thinking if this commit looks good I can make a second commit with the next part.

@@ -584,8 +584,8 @@ impl<'a> TraitDef<'a> {
defaultness: ast::Defaultness::Final,
generics: Generics::default(),
where_clauses: (
ast::TyAliasWhereClause::default(),
ast::TyAliasWhereClause::default(),
ast::TyAliasWhereclause::default(),
Copy link
Member

Choose a reason for hiding this comment

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

A global "Clause" -> "clause" rename is going to give a ton of mistakes like this. Can you fix those?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I did not notice that, will be fixing it :)

@@ -2003,9 +2003,9 @@ dependencies = [

[[package]]
Copy link
Member

Choose a reason for hiding this comment

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

Can you drop the Cargo.lock change from these changes?

@compiler-errors
Copy link
Member

reexport Clause's variants from the clause module

Where is the clause module?

@sladyn98
Copy link
Contributor Author

sladyn98 commented Feb 2, 2023

reexport Clause's variants from the clause module

Where is the clause module?

I was actually searching on how to import this module but I could not find any examples

@compiler-errors
Copy link
Member

I was actually searching on how to import this module but I could not find any examples

It's not a module. You need to make a module called clause and re-export the Clause enum's variants in that module.

@compiler-errors
Copy link
Member

Marking this as waiting-on-author.

@rustbot author
r? @oli-obk

@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 Feb 2, 2023
@rustbot rustbot assigned oli-obk and unassigned estebank Feb 2, 2023
@sladyn98
Copy link
Contributor Author

sladyn98 commented Feb 7, 2023

You need to make a module called clause and re-export the Clause enum's variants in that module.

@compiler-errors Under which folder do I create this module and where would I get a list of all of the variants.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 7, 2023

@sladyn98 unfortunately most reviewers won't have the time for the kind of hands-on mentoring you're asking for. We cannot teach Rust the language at the same time as teaching how the Rust compiler works. The rust compiler itself is not a good place to learn the language, as it contains a lot of unstable Rust features, complex code for various reasons and old code that we never updated. I would recommend working on smaller projects first to build up Rust expertise and then coming back to working on rustc.

We do have smaller, self-contained projects under rust-lang ownership like the cargo-bisect-rustc crate, and there are many community projects asking for help on this-week-in-rust.

I'm sorry for cutting off this PR instead of helping you finish it, but I believe it would not be a good experience for you and the reviewer, so i'll close it now to avoid you putting a lot of work into it only to have it get closed later.

Thank you for taking up the work here to do this cleanup and I hope we'll see you again in the future

@oli-obk oli-obk closed this Feb 7, 2023
@sladyn98
Copy link
Contributor Author

sladyn98 commented Feb 8, 2023

Thanks :) No problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Track perf regression "Branch Clause from Predicate #104846"
5 participants