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.

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

Split astconv.rs into its own submodule #75711

Merged
merged 1 commit into from
Aug 22, 2020

Conversation

CohenArthur
Copy link
Contributor

Fixes #67418

This changed induced a few changes across the Type checker, but only there. Mostly, it was just renaming Self:: into something else to call specific methods from a subtrait instead of having a 2500+ lines one.

I split up the astconv.rs file into its own module. This way, directives such as

use crate::astconv::AstConv;

are still valid, and doing

use crate::astconv::{AstConv, AstConvGeneric};

is possible

(instead of having two modules, one named astconv_generic.rs for example and astconv.rs)

I'm not entirely sure that the name AstConvGeneric is a good one. However, only methods related to lifetimes or generics have been moved over to this module. Sorry about the large diff.

I'd be very happy to make any correction you deem necessary.

r? @oli-obk

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 19, 2020
@CohenArthur
Copy link
Contributor Author

Might I add: the file is still big, at 2.6k LoC. Splitting the error methods (complain_*) is harder and the file would still be over 2k LoC. I really didn't want to break anything do I avoided doing that, but I'd be willing to if you think it'd be better.

@CohenArthur CohenArthur force-pushed the split-up-astconv branch 2 times, most recently from 0c9bd98 to 19c7072 Compare August 20, 2020 12:31
@CohenArthur CohenArthur force-pushed the split-up-astconv branch 2 times, most recently from 85bf7ad to 49c31d1 Compare August 20, 2020 13:11
To separate the astconv.rs file, I split it into its own module with a
subtrait called GenericAstConv. This subtrait handles methods related to
generics, be it types or lifetimes.

typeck: Add bounds module and Bounds struct

bounds: Run fmt, add documentation

generic_astconv: Add subtrait GenericAstConv

Some methods of AstConv deal exclusively with Generics. Therefore, it
makes sense to have them in their own trait. Some other methods from
AstConv might be added to it later

generic_astconv: Add more methods from AstConv

Add check_generic_arg_count_for_call() and check_generic_arg_count()

astconv: Add module for clarity

generic: Rename GenericAstConv -> AstConvGeneric

generic: add more methods to AstConvGeneric

astconv: Remove AstConvGeneric trait, add impl dyn AstConv in other
module

astconv: Add errors module to handle AstConv complaints

fmt: format code in astconv/

astconv: Remove old file

astconv: Fix visibility on GenericArgPosition

astconv: Fix visibility on GenericArgPosition

astconv: Fix function visibility on other originally private functions
@oli-obk
Copy link
Contributor

oli-obk commented Aug 21, 2020

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Aug 21, 2020

📌 Commit e6642e4 has been approved by oli-obk

@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 21, 2020
@CohenArthur
Copy link
Contributor Author

@oli-obk thanks to you for the helpful reviews !

@Dylan-DPC-zz
Copy link

@bors p=1

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 22, 2020
Rollup of 12 pull requests

Successful merges:

 - rust-lang#75705 (Move to intra-doc links for /library/core/src/intrinsics.rs)
 - rust-lang#75711 (Split `astconv.rs` into its own submodule)
 - rust-lang#75718 (Don't count variants/fields/consts/associated types in doc-coverage doc examples)
 - rust-lang#75725 (Use intra-doc-links in `alloc`)
 - rust-lang#75745 (Remove duplication in `fold_item`)
 - rust-lang#75753 (Another motivation for CFG: return-oriented programming)
 - rust-lang#75769 (Minor, remove double nesting of a test module)
 - rust-lang#75771 (Extend normalization in const-eval-query-stack test)
 - rust-lang#75781 (More inline asm register name fixups for LLVM)
 - rust-lang#75782 (Convert core/src/str/pattern.rs to Intra-doc links)
 - rust-lang#75787 (Use intra-doc-links in `core::ops::*`)
 - rust-lang#75788 (MIR call terminator represents diverging calls too)

Failed merges:

 - rust-lang#75773 (Introduce expect snapshot testing library into rustc)

r? @ghost
@bors
Copy link
Contributor

bors commented Aug 22, 2020

⌛ Testing commit e6642e4 with merge e674422...

@bors bors merged commit 6cb98e3 into rust-lang:master Aug 22, 2020
@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
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Split up astconv.rs
6 participants