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

Rename TyVariants and variants #53581

Merged
merged 9 commits into from
Aug 22, 2018
Merged

Conversation

varkor
Copy link
Member

@varkor varkor commented Aug 22, 2018

  • Rename TypeVariants to TyKind.
  • Remove the Ty prefix from each one of its variants (plus the identically-named variants of PrimTy).
  • Rename ty::Slice to ty::List.

The new names look cleaner.

r? @eddyb

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 22, 2018
@@ -53,13 +53,13 @@ pub enum Def {
Existential(DefId),
/// `type Foo = Bar;`
TyAlias(DefId),
TyForeign(DefId),
Foreign(DefId),
TraitAlias(DefId),
AssociatedTy(DefId),
/// `existential type Foo: Bar;`
AssociatedExistential(DefId),
PrimTy(hir::PrimTy),
Copy link
Member

Choose a reason for hiding this comment

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

Hm, maybe drop Ty here too? And in the other cases perhaps?

Copy link
Member

Choose a reason for hiding this comment

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

No, this is "primitive type". Def contains both types and values.

@eddyb
Copy link
Member

eddyb commented Aug 22, 2018

cc @nikomatsakis

@@ -53,13 +53,13 @@ pub enum Def {
Existential(DefId),
/// `type Foo = Bar;`
TyAlias(DefId),
TyForeign(DefId),
Foreign(DefId),
Copy link
Member

Choose a reason for hiding this comment

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

This seems suboptimal... what if you first rename this to ForeignTy and then do the other renames?

TraitAlias(DefId),
AssociatedTy(DefId),
/// `existential type Foo: Bar;`
AssociatedExistential(DefId),
PrimTy(hir::PrimTy),
TyParam(DefId),
Param(DefId),
Copy link
Member

Choose a reason for hiding this comment

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

We probably want to keep this the way it is, assuming we need to tell apart ty params from const ones.

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[00:45:55] ....................................................................................................
[00:45:58] ....................................................................................................
[00:46:01] ....................................................................................................
[00:46:04] ....................................................................................................
[00:46:07] ..................................................................................F.................
[00:46:13] ....................................................................................................
[00:46:17] ....................................................................................................
[00:46:19] ......................................i.............................................................
[00:46:22] ........................................................................................i.i..ii.....
---
[00:47:04] diff of stderr:
[00:47:04] 
[00:47:04] 2   --> $DIR/issue-46332.rs:19:5
[00:47:04] 3    |
[00:47:04] 4 LL |     TyUInt {};
[00:47:04] -    |     ^^^^^^ did you mean `TyUint`?
[00:47:04] 6 
[00:47:04] 7 error: aborting due to previous error
[00:47:04] 8 
[00:47:04] 
[00:47:04] 
[00:47:04] 
[00:47:04] The actual stderr differed from the expected stderr.
[00:47:04] Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/issues/issue-46332/issue-46332.stderr
[00:47:04] To update references, rerun the tests and pass the `--bless` flag
[00:47:04] To only update this specific test, also pass `--test-args issues/issue-46332.rs`
[00:47:04] error: 1 errors occurred comparing output.
[00:47:04] status: exit code: 1
[00:47:04] status: exit code: 1
[00:47:04] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/u "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-5.0/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "5.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[00:47:04] 
[00:47:04] 
[00:47:04] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:47:04] Build completed unsuccessfully in 0:03:05
[00:47:04] Build completed unsuccessfully in 0:03:05
[00:47:04] Makefile:58: recipe for target 'check' failed
[00:47:04] make: *** [check] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0cbec385
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:0cccfb82:start=1534903907850094846,finish=1534903907858103470,duration=8008624
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0f032a19
$ ln -s . checkout && fo

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@bors
Copy link
Contributor

bors commented Aug 22, 2018

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

@@ -53,13 +53,13 @@ pub enum Def {
Existential(DefId),
/// `type Foo = Bar;`
TyAlias(DefId),
Foreign(DefId),
TyForeign(DefId),
Copy link
Member

Choose a reason for hiding this comment

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

I prefer ForeignTy, specifically, just like AssociatedTy below.

fn ty_param_secret(&self);
}

mod m {
struct Priv;

impl ::Arr0 for [Priv; 0] { fn arr0_secret(&self) {} }
impl ::TyParam for Option<Priv> { fn ty_param_secret(&self) {} }
impl ::Param for Option<Priv> { fn ty_param_secret(&self) {} }
Copy link
Member

Choose a reason for hiding this comment

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

Hehe you don't need to change this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@bors
Copy link
Contributor

bors commented Aug 22, 2018

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

@eddyb
Copy link
Member

eddyb commented Aug 22, 2018

@varkor r=me when you rebase

@varkor
Copy link
Member Author

varkor commented Aug 22, 2018

@bors r=eddyb p=10

(This bitrots very quickly.)

@bors
Copy link
Contributor

bors commented Aug 22, 2018

📌 Commit 71722b9 has been approved by eddyb

@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 22, 2018
@bors
Copy link
Contributor

bors commented Aug 22, 2018

⌛ Testing commit 71722b9 with merge b75b047...

bors added a commit that referenced this pull request Aug 22, 2018
Rename TyVariants and variants

- Rename `TypeVariants` to `TyKind`.
- Remove the `Ty` prefix from each one of its variants (plus the identically-named variants of `PrimTy`).
- Rename `ty::Slice` to `ty::List`.

The new names look cleaner.

r? @eddyb
@bors
Copy link
Contributor

bors commented Aug 22, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing b75b047 to master...

@bors bors merged commit 71722b9 into rust-lang:master Aug 22, 2018
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #53581!

Tested on commit b75b047.
Direct link to PR: #53581

💔 clippy-driver on windows: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra).
💔 clippy-driver on linux: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Aug 22, 2018
Tested on commit rust-lang/rust@b75b047.
Direct link to PR: <rust-lang/rust#53581>

💔 clippy-driver on windows: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra).
💔 clippy-driver on linux: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra).
@varkor varkor deleted the tyvariants-rename branch August 22, 2018 20:04
matthiaskrgr added a commit to matthiaskrgr/rust-clippy that referenced this pull request Aug 22, 2018
@petrochenkov petrochenkov removed their assignment Aug 22, 2018
flip1995 added a commit to rust-lang/rust-clippy that referenced this pull request Aug 23, 2018
nikic added a commit to nikic/rustc-guide that referenced this pull request Oct 26, 2018
mark-i-m pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Oct 26, 2018
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.

6 participants