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

Update rust.md et al with proper use of newly required explicit self parameter #5085

Closed
pnkfelix opened this issue Feb 22, 2013 · 3 comments
Closed
Milestone

Comments

@pnkfelix
Copy link
Member

While trying to explore how the rust trait system deals with method name collisions 1, I had some difficulty even getting started with simple examples. So I decided to consult rust.md.

However, transcribing the current examples out of rust.md and into a new source file does not work "out of the box", because the examples use legacy syntax and cause warnings to be issued 2. When I attempted to fix the transcribed examples to get rid of the warnings, more problems arose.

Partial list of issues in current trait examples:

  1. The first trait Shape example should have explicit self parameters.
  2. The draw_twice example complains due to use of moved value, sh
  3. The impl Shape for Circle example needs an explicit self
  4. The Object types trait Printable example needs an example self parameter 3

etc


I argue that it is important to fix these examples, since if the examples are not corrected, it is ambiguous to the reader what format a self parameter is supposed to take. E.g. I initially attempted to correct the problem in this manner (adding a named parameter for the self argument, by analogy with how one puts in names for other method arguments declared on the trait):

trait Shape           { fn draw(     self,    Surface); }
impl Shape for Circle { fn draw(this:self, s: Surface) { do_draw_circle(s, self); } }

but that (obviously?) is not the correct fix, and the compiler complains:

trait-play.rs:29:37: 29:41 error: use of undeclared type name `self`
trait-play.rs:29 impl Shape for Circle { fn draw(this:self, s: Surface) { do_draw_circle(s, self); } }
                                                      ^~~~
error: aborting due to previous error

(I am aware that the correct fix is to leave the name off of the self parameter; the point is that the documentation needs to reflect this.)

Footnotes

trait Printable {
  fn to_str(self) -> ~str;
}

impl Printable for int {
  fn to_str(self) -> ~str { int::to_str(self) }
}

yields:

error: internal compiler error: methods with by-value self should not be called on objects

Footnotes

  1. I infer method name collisions to be the motivation for using the syntax traitname::f() for static method invocation, and not typeparameter::f(); but I wanted to test that name collisions are indeed resolved in some manner.

  2. I assume we must be compiling the extracted code samples in some sort of legacy compiler mode? (Maybe these examples should be forcibly tested in a vanilla mode.) Or maybe the issue is that we ignore warnings when compiling the documentation? That should be something one has to opt-into in a written example, though, if possible.

  3. Worse, after adding "the obvious" self parameter, the code no longer compiles:

@pnkfelix
Copy link
Member Author

Regarding footnote [^3], I don't know whether that is a bug elsewhere or what. One can workaround the issue by replacing the self parameter with &self and the use of self in the body with *self, but that does not seem like the right approach here, since the code worked before we added the print(a: @Printable) method, and thus adding the latter should not force us to make changes to our existing Trait definitions, right?

@pnkfelix
Copy link
Member Author

Also, regarding footnote [^3], that is related to issue #4776.

@graydon
Copy link
Contributor

graydon commented Apr 29, 2013

I believe this is fixed now; reopen if you see more of these?

@graydon graydon closed this as completed Apr 29, 2013
bors added a commit to rust-lang-ci/rust that referenced this issue May 2, 2020
Split up `needless_range_loop` ui test

Part of rust-lang#2038

changelog: none
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

No branches or pull requests

2 participants