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

Don't import integer and float modules, use assoc consts 2 #70857

Merged
merged 9 commits into from
Apr 7, 2020

Conversation

faern
Copy link
Contributor

@faern faern commented Apr 6, 2020

Follow up to #70777. I missed quite a lot of places. Partially because I wanted to keep the size of the last PR down, and partially because my regexes were not good enough :)

r? @dtolnay

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 6, 2020
Some(*ordinal as usize)
} else {
let msg = format!("ordinal value in `link_ordinal` is too large: `{}`", &ordinal);
tcx.sess
.struct_span_err(attr.span, &msg)
.note("the value may not exceed `std::usize::MAX`")
.note("the value may not exceed `usize::MAX`")
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 a user facing change and not just an internal code update. And since we want new code to use the new assoc consts this is important to update to have a consistent message to users.

@faern
Copy link
Contributor Author

faern commented Apr 6, 2020

This still fails an ./x.py test run... I was too lazy and did not wait for my local run to finish (since it takes ages) before submitting the PR. Currently working on that. So there will be more stuff pushed.

@faern faern force-pushed the use-assoc-int-float-consts branch from 029f0ba to f7778d3 Compare April 6, 2020 22:52
@faern
Copy link
Contributor Author

faern commented Apr 6, 2020

Hopefully fixed now. The result of that test came in far sooner than I thought.

Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks!

@dtolnay
Copy link
Member

dtolnay commented Apr 6, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Apr 6, 2020

📌 Commit f7778d3 has been approved by dtolnay

@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 Apr 6, 2020
Centril added a commit to Centril/rust that referenced this pull request Apr 7, 2020
…=dtolnay

Don't import integer and float modules, use assoc consts 2

Follow up to rust-lang#70777. I missed quite a lot of places. Partially because I wanted to keep the size of the last PR down, and partially because my regexes were not good enough :)

r? @dtolnay
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 7, 2020
…ievink

Rollup of 5 pull requests

Successful merges:

 - rust-lang#70201 (Small tweaks in ToOwned::clone_into)
 - rust-lang#70762 (Miri leak check: memory reachable through globals is not leaked)
 - rust-lang#70846 (Keep codegen units unmerged when building compiler builtins)
 - rust-lang#70854 (Use assoc int submodules)
 - rust-lang#70857 (Don't import integer and float modules, use assoc consts 2)

Failed merges:

r? @ghost
@bors bors merged commit 89d661f into rust-lang:master Apr 7, 2020
@faern faern deleted the use-assoc-int-float-consts branch April 16, 2020 21:54
@faern faern mentioned this pull request Apr 20, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 21, 2020
Use assoc int consts3

Define module level int consts with associated constants instead of `min_value()` and `max_value()`. So the code become consistent with what the docs recommend etc. Seems natural.

Also remove the last usages of the int module constants from this repo (except src/test/ directory which I have still not really done anything in). Some places were missed in the previous PRs because the code uses `crate::<IntTy>` to reach the constants.

This is a continuation of rust-lang#70857

r? @dtolnay
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 21, 2020
Use assoc int consts3

Define module level int consts with associated constants instead of `min_value()` and `max_value()`. So the code become consistent with what the docs recommend etc. Seems natural.

Also remove the last usages of the int module constants from this repo (except src/test/ directory which I have still not really done anything in). Some places were missed in the previous PRs because the code uses `crate::<IntTy>` to reach the constants.

This is a continuation of rust-lang#70857

r? @dtolnay
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Apr 22, 2020
Use assoc int consts3

Define module level int consts with associated constants instead of `min_value()` and `max_value()`. So the code become consistent with what the docs recommend etc. Seems natural.

Also remove the last usages of the int module constants from this repo (except src/test/ directory which I have still not really done anything in). Some places were missed in the previous PRs because the code uses `crate::<IntTy>` to reach the constants.

This is a continuation of rust-lang#70857

r? @dtolnay
bors added a commit to rust-lang/cargo that referenced this pull request Apr 28, 2020
Use associated constants directly on primitive types instead of modules

This PR is in no way critical. It's more of a code cleanup. It comes as a result of me making rust-lang/rust#70857 and search-and-replacing all usage of the soft-deprecated ways of reaching primitive type constants.

It makes the code slightly shorter, that's basically it. And showcases the recommended way of reaching these consts on new code :)
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.

4 participants