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

Remove mentions of the old tilde owned pointer syntax #25085

Merged
merged 6 commits into from
May 11, 2015
Merged

Remove mentions of the old tilde owned pointer syntax #25085

merged 6 commits into from
May 11, 2015

Conversation

carols10cents
Copy link
Member

There were still some mentions of ~[T] and ~T, mostly in comments and debugging statements. I tried to do my best to preserve meaning, but I might have gotten some wrong-- I'm happy to fix anything :)

Also remove comments that reference the unique_type_id HEAP_VEC_BOX
metadata, which was removed in 3e62637 and the unique_type_id GC_BOX
metadata, which was removed in 8a91d33.
@rust-highfive
Copy link
Contributor

r? @Aatch

(rust_highfive has picked a reviewer for you, use r? to override)

@@ -1125,7 +1125,7 @@ impl<'d,'t,'tcx,TYPER:mc::Typer<'tcx>> ExprUseVisitor<'d,'t,'tcx,TYPER> {
// that case we can adjust the length of the
// original vec accordingly, but we'd have to
// make trans do the right thing, and it would
// only work for `~` vectors. It seems simpler
// only work for `Vec`s. It seems simpler
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically this would be refererring to Box<[T]> types, as Vec isn't a built-in type that the compiler should know about.

@Aatch
Copy link
Contributor

Aatch commented May 4, 2015

Seems ok to me, other than those small things I mentioned. Though I'm not sure about some of the other places where the choice of ~[T] seems arbitrary. It's possible that the new coherence and impl rules make the original choice of ~[T] irrelevant.

Techncially, the closest equivalent to ~[T] is Box<[T]> not Vec<T>, even though user code suggests the opposite.

@@ -202,7 +200,7 @@ impl<'tcx> TypeMap<'tcx> {
}
},
ty::ty_uniq(inner_type) => {
unique_type_id.push('~');
unique_type_id.push_str("Box ");
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't the lowercase version for the box syntax make more sense here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have no idea, sounds plausible to me! Change incoming.

@carols10cents
Copy link
Member Author

Thank you @Aatch and @oli-obk! I've added new commits to address your comments-- I'm happy to squash these commits if these changes are good or remove them if they aren't or make additional changes if necessary.

@steveklabnik
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Collaborator

bors commented May 10, 2015

📌 Commit abc0017 has been approved by steveklabnik

@steveklabnik
Copy link
Member

@bors: -rollup

@steveklabnik
Copy link
Member

let's keep this on its own since it modifies tests and the queue isn't very full

steveklabnik added a commit to steveklabnik/rust that referenced this pull request May 10, 2015
…teveklabnik

There were still some mentions of `~[T]` and `~T`, mostly in comments and debugging statements. I tried to do my best to preserve meaning, but I might have gotten some wrong-- I'm happy to fix anything :)
bors added a commit that referenced this pull request May 10, 2015
@steveklabnik
Copy link
Member

@bors: rollup-

Manishearth added a commit to Manishearth/rust that referenced this pull request May 11, 2015
…teveklabnik

There were still some mentions of `~[T]` and `~T`, mostly in comments and debugging statements. I tried to do my best to preserve meaning, but I might have gotten some wrong-- I'm happy to fix anything :)
@bors
Copy link
Collaborator

bors commented May 11, 2015

⌛ Testing commit abc0017 with merge 7334518...

bors added a commit that referenced this pull request May 11, 2015
There were still some mentions of `~[T]` and `~T`, mostly in comments and debugging statements. I tried to do my best to preserve meaning, but I might have gotten some wrong-- I'm happy to fix anything :)
@bors bors merged commit abc0017 into rust-lang:master May 11, 2015
@carols10cents carols10cents deleted the remove-old-tilde branch May 11, 2015 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants