Skip to content

fix #24968 more friend error message for Self in fn args #25096

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

Conversation

XuefengWu
Copy link
Contributor

fix #24968
report more friendly error message for Self when fn args

@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nrc (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see CONTRIBUTING.md for more information.

@arielb1
Copy link
Contributor

arielb1 commented May 5, 2015

Your message sounds like a type error. I would prefer one in the style of "use of Self outside of an impl".

XuefengWu and others added 2 commits May 10, 2015 17:09
I've been working with some archives generated by MSVC's `lib.exe` tool lately,
and it looks like the embedded name of the members in those archives sometimes
have slahes in the name (e.g. `foo/bar/baz.obj`). Currently the compiler chokes
on these paths as it assumes that each file in the archive is only the filename
(which is what unix does).

This commit interprets the name of each file in all archives as a path and then
only uses the `file_name` portion of the path to extract the file to a separate
location and then reassemble it back into a new archive later. Note that
duplicate filenames are already handled, so this won't introduce any conflicts.
@nrc
Copy link
Member

nrc commented May 13, 2015

Hi @XuefengWu, thanks for the PR and sorry it took so long for me to get to it - I was on vacation last week.

A high level thing is that this PR needs a test to check that we get the right error messages. You should add a test to the compile-fail directory which checks the message.

More comments inline.

maybe_qself.is_none() &&
path.segments[0].identifier.name == self_type_name;
let msg = if is_invalid_self_type_name {
"use of Self outside of an impl".to_string()
Copy link
Member

Choose a reason for hiding this comment

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

Self can also be used in a trait, so the message should include that. Also, you could add backticks around "Self".

mdinger and others added 22 commits May 13, 2015 00:06
We don't have any pending snapshot-requiring changes. Tests which
continue to be ignored are those that are broken by codegen changes.
Also change several error messages to refer to "items" rather than
"methods", since associated items that require resolution during type
checking are not always methods.
This commit is an implementation of [RFC 1040][rfc] which is a redesign of the
currently-unstable `Duration` type. The API of the type has been scaled back to
be more conservative and it also no longer supports negative durations.

[rfc]: https://github.com/rust-lang/rfcs/blob/master/text/1040-duration-reform.md

The inner `duration` module of the `time` module has now been hidden (as
`Duration` is reexported) and the feature name for this type has changed from
`std_misc` to `duration`. All APIs accepting durations have also been audited to
take a more flavorful feature name instead of `std_misc`.

Closes rust-lang#24874
This also updates the error messages for both. For E0066, it removes mention
of "managed heap", which was removed in 8a91d33. For E0069, I just tweaked
the wording to make it a bit more explicit.
See:
https://sourceware.org/bugzilla/show_bug.cgi?id=4887#c9
https://bugs.freedesktop.org/show_bug.cgi?id=65681

I just noticed this while talking to someone who was using
`os.environ['FOO'] = 'BAR'` in Python and since I'm learning Rust, I
was curious if it did anything special here.  It looks like Rust has
an internal mutex, which helps for apps that are pure Rust, but it
will be an evil trap for someone later adding in native code (apps
like Servo and games will be at risk).

Java got this right by disallowing `setenv()` from the start.

I suggest Rust program authors only use `setenv()` early in main.
…uity_message, r=nikomatsakis

This fixes rust-lang#24922 and rust-lang#25017, and reduces the number of error messages that talk about "methods" when associated constants rather than methods are involved.

I will admit that I haven't thought very carefully about the error messages. My goal has been to make more of the messages technically correct in all situations, and to avoid ICEs. But in some cases we could probably talk specifically about "methods" rather than "items".
…, r=brson

I've been working with some archives generated by MSVC's `lib.exe` tool lately,
and it looks like the embedded name of the members in those archives sometimes
have slahes in the name (e.g. `foo/bar/baz.obj`). Currently the compiler chokes
on these paths as it assumes that each file in the archive is only the filename
(which is what unix does).

This commit interprets the name of each file in all archives as a path and then
only uses the `file_name` portion of the path to extract the file to a separate
location and then reassemble it back into a new archive later. Note that
duplicate filenames are already handled, so this won't introduce any conflicts.
…e-not-threadsafe, r=alexcrichton

See:
https://sourceware.org/bugzilla/show_bug.cgi?id=4887#c9
https://bugs.freedesktop.org/show_bug.cgi?id=65681

I just noticed this while talking to someone who was using
`os.environ['FOO'] = 'BAR'` in Python and since I'm learning Rust, I
was curious if it did anything special here (and the answer appears to
be no).

Java got this right by disallowing `setenv()` from the start.
…ichton

We don't have any pending snapshot-requiring changes. Closes rust-lang#20184.

Works toward rust-lang#3965.
rustbook throws errors if the `_book` folder exists already. Common if you build twice in a row. Identical to https://github.com/steveklabnik/rustbook/issues/20
steveklabnik and others added 20 commits May 14, 2015 20:30
…-typo, r=steveklabnik

I fixed the typo of the value of e in the memory tables. It is a reference to d, and so it should contain the memory location of d. I also fixed the incorrectly formatted tables so they display properly in html pages.
Every time I profile my code I find something new to add #[inline] to. (:
It seems to refer to something that used to exist, but got moved, and then not everything got cleaned up.
The assume intrinsic has a strong, negative impact on compile times, so
we're currently only using it in places where LLVM can simplify it to
nonnull metadata on a load intruction. Unfortunately a recent change
that fixed invalid assume calls introduce new assume calls for which
this simplification can not happen, leading to a massive regression in
compile times in certain cases.

Moving the assumptions from the middle of the function to the beginning
allows the simplification to happen again, bringing compile times back
to their old levels.

Fixes rust-lang#25393
Also start factoring out an API for compiler tools to use and fix a bug that was preventing DXR indexing Rust properly.

r? @huonw
The assume intrinsic has a strong, negative impact on compile times, so
we're currently only using it in places where LLVM can simplify it to
nonnull metadata on a load intruction. Unfortunately a recent change
that fixed invalid assume calls introduce new assume calls for which
this simplification can not happen, leading to a massive regression in
compile times in certain cases.

Moving the assumptions from the middle of the function to the beginning
allows the simplification to happen again, bringing compile times back
to their old levels.

Fixes rust-lang#25393
@XuefengWu
Copy link
Contributor Author

@nrc The change commit is: XuefengWu@eac661c

@nrc
Copy link
Member

nrc commented May 16, 2015

@XuefengWu something has gone wrong with the Git history here. I'm afraid we can't merge this as is. On local branches, it is usually better to use rebase rather than merge (merge can work too, but it is easier to get wrong and the history doesn't look as nice). To fix this, I would recommend starting a new branch from master and cherry-picking the commits from this branch. Then force push that branch to this remote: git push origin new_branch_name:24968_err_msg_parse_self_type. You can use git rebase -i to squash commits together or to reword commit messages (I note that the last commit here has a typo in 'keyword'). Let me know if you need more help with the git stuff, or ask on #rust-internals.

@nrc
Copy link
Member

nrc commented May 16, 2015

r=me with the git history sorted and the two nits fixed.

@nrc
Copy link
Member

nrc commented May 16, 2015

Closing in favour of #25485

@nrc nrc closed this May 16, 2015
@XuefengWu
Copy link
Contributor Author

@nrc I copied a new PR #25485

Should I close this PR?

bors added a commit that referenced this pull request May 16, 2015
fix #24968
report more friendly error message for Self when fn args
copy from #25096
r? @nrc  @arielb1
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.

Wrong error message when using Self in some places