Skip to content

RFC: Remove default ToStr implementations for @ and ~ pointers #4869

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

Closed
pkgw opened this issue Feb 10, 2013 · 5 comments
Closed

RFC: Remove default ToStr implementations for @ and ~ pointers #4869

pkgw opened this issue Feb 10, 2013 · 5 comments
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@pkgw
Copy link
Contributor

pkgw commented Feb 10, 2013

Right now, libcore contains default implementations of the ToStr trait for pointers to ToStr-able implementations:

impl<A: ToStr> @A: ToStr { basically: "@" + (*self).to_str () }

To the best of my understanding, it isn't possible to override these implementations (see issue #4851). If this is the case, I argue that these default implementations are far too restrictive and should be removed.

Consider a case where you have some stack object with a meaningful ToStr. If you have a pointer to that object, I'd say that it's much more plausible that you'll want that pointer to yield the same string, not "@" + that string. If you don't want that, you at least want control over the stringification.

It only makes sense to require the current semantics if ToStr is defined as behaving like Python's repr functionality. But it isn't. (For instance, quite reasonably (s: str).to_str() = s, not "\"" + escape(s) + "\"".)

If I comment out the default implementations for pointers and the tests specific to those pointers, I can rebuild all of Rust and all of the remaining tests pass.

If these default implementations are removed, then (~[1, 2, 3]).to_str() starts evaluating to "[1, 2, 3]" rather than "~[1, 2, 3]". I claim that this is desirable.

@Dretch
Copy link
Contributor

Dretch commented Feb 10, 2013

+1. I think it is inconsistent that (currently) @T.to_str() and ~T.to_str() include the pointer sigil in the str, but &T.to_str() does not.

@brson
Copy link
Contributor

brson commented Feb 11, 2013

+1

1 similar comment
@catamorphism
Copy link
Contributor

+1

@pkgw
Copy link
Contributor Author

pkgw commented Feb 19, 2013

Should I put together a patch and a pull request implementing this, then?

@catamorphism
Copy link
Contributor

@pkgw Sure -- normally we try to get consensus from the core team, but this seems like a pretty benign change and there haven't been objections..

bors added a commit that referenced this issue Feb 28, 2013
See issue #4869. I'm not quite sure what constitutes "consensus from the core team" (cf. discussion in the issue), but this at least demonstrates that the proposed change is pretty straightforward.

After this change, there are no new test failures. I've un-ignored the `to_str` vectors test; it's not at all obvious to me why it'd be problematic, and it passes on my Linux machine.
@brson brson closed this as completed in a712d82 Mar 4, 2013
calebcartwright added a commit to calebcartwright/rust that referenced this issue Jul 26, 2021
* Updating outdated links

Updating the links to the docs and source code for `ast.rs`. Seems like it was moved to a new crate at some point.

* Updating more outdated links

This time, the links to the `fmt-rfcs` repository, which is now owned by
`rust-dev-tools` (although GitHub was redirecting anyway).

* Update Contributing.md

Co-authored-by: Caleb Cartwright <calebcartwright@users.noreply.github.com>

Co-authored-by: Caleb Cartwright <calebcartwright@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

No branches or pull requests

4 participants