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

Refactor type_of for constants #70478

Merged
merged 1 commit into from
Mar 28, 2020
Merged

Refactor type_of for constants #70478

merged 1 commit into from
Mar 28, 2020

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Mar 27, 2020

If I have to look at this function for a few hours I want it to at least look good.

r? @varkor

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 27, 2020
@lcnr lcnr changed the title Refactor type of for constants Refactor type_of for constants Mar 27, 2020
@@ -194,6 +194,9 @@ pub enum GenericArg {
/// `'a` in `Foo<'a>`
Lifetime(Lifetime),
/// `Bar` in `Foo<Bar>`
///
/// This is also used for const parameters
/// which are either `()` or a path.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is actually not correct rn, as () currently requires braces

Copy link
Member

@varkor varkor left a comment

Choose a reason for hiding this comment

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

r=me after comment change and CI passes.

Comment on lines 198 to 199
/// This is also used for const parameters
/// which are either `()` or a path.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// This is also used for const parameters
/// which are either `()` or a path.
/// This is also used for const parameters that are either `()` or a path,
/// since we can't tell until type-checking (or sometimes name
/// resolution), whether they refer to types or consts. This will be
/// converted to consts later if we find out that's what they should have
/// been all along.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this comment completely from this PR for now.

As I am currently working on #67075 and #66615 I will update the comment there. (Even if it will still take a few years until I get that done 😆 )

@varkor varkor added the F-const_generics `#![feature(const_generics)]` label Mar 27, 2020
@varkor
Copy link
Member

varkor commented Mar 27, 2020

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 27, 2020

📌 Commit c339b2e 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 Mar 27, 2020
Centril added a commit to Centril/rust that referenced this pull request Mar 27, 2020
Refactor type_of for constants

If I have to look at this function for a few hours I want it to at least look good.

r? @varkor
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 28, 2020
Rollup of 5 pull requests

Successful merges:

 - rust-lang#70345 (Remove `no_integrated_as` mode.)
 - rust-lang#70434 (suggest `;` on expr `mac!()` which is good as stmt `mac!()`)
 - rust-lang#70457 (non-exhastive diagnostic: add note re. scrutinee type)
 - rust-lang#70478 (Refactor type_of for constants)
 - rust-lang#70480 (clarify hir_id <-> node_id method names)

Failed merges:

r? @ghost
@bors bors merged commit 5b68f9c into rust-lang:master Mar 28, 2020
@lcnr lcnr deleted the refactor-type_of branch April 27, 2020 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

4 participants