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

Point at local similarly named element and tweak references to variants #65421

Merged
merged 1 commit into from
Oct 28, 2019

Conversation

estebank
Copy link
Contributor

Partially address #65386.

@rust-highfive
Copy link
Collaborator

r? @davidtwco

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 15, 2019
@estebank
Copy link
Contributor Author

cc @petrochenkov @Centril

@rust-highfive

This comment has been minimized.

@bors

This comment has been minimized.

@Centril
Copy link
Contributor

Centril commented Oct 15, 2019

r? @petrochenkov (to make sure there's consensus due to #65386 (comment) and because Vadim knows resolve well ^^)

src/librustc/hir/def.rs Outdated Show resolved Hide resolved
src/librustc_resolve/diagnostics.rs Outdated Show resolved Hide resolved
src/librustc_resolve/late/diagnostics.rs Outdated Show resolved Hide resolved
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 15, 2019
::: $SRC_DIR/libcore/cmp.rs:LL:COL
|
LL | pub macro Eq($item:item) { /* compiler built-in */ }
| ---------------------------------------------------- similarly named derive macro `Eq` defined here
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems to me that for some proc and derive macros the new output might be more confusing than helpful for new comers ("how do I do that"), but at the same time they'd be nice to have as well for everyone else.

::: $SRC_DIR/libcore/result.rs:LL:COL
|
LL | Ok(#[stable(feature = "rust1", since = "1.0.0")] T),
| --------------------------------------------------- similarly named tuple variant `Ok` defined here
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Somewhat unexpectedly (but not unforeseeably), this change now makes more obvious that attribute annotations are part of the public documentation and will be more visible than currently. In some cases like this one it might make sense to tweak them "out of the way" to improve readability while keeping the same semantic meaning.

@estebank
Copy link
Contributor Author

estebank commented Oct 16, 2019

Current output for the original report being addressed:
Screen Shot 2019-10-16 at 9 34 14 PM

@estebank estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 16, 2019
@rust-highfive

This comment has been minimized.

@rust-highfive

This comment has been minimized.

src/librustc_resolve/diagnostics.rs Outdated Show resolved Hide resolved
src/librustc_resolve/diagnostics.rs Outdated Show resolved Hide resolved
src/librustc_resolve/late.rs Outdated Show resolved Hide resolved
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 17, 2019
@estebank estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 17, 2019
@petrochenkov
Copy link
Contributor

We cannot land this with disabled tests.
If the tests failed on x86 linux, then it's very likely that the they will fail on some targets on CI, and then on other targets that people use but which are not on CI.

The change causing non-reproducible output needs to be either removed or investigated/fixed to remove the source of non-reproducibility.

Beside that the PR looks good.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 27, 2019
Point at the span for the definition of ADTs internal to the current
crate.

Look at the leading char of the ident to determine whether we're
expecting a likely fn or any of a fn, a tuple struct or a tuple variant.

Turn fn `add_typo_suggestion` into a `Resolver` method.
@estebank
Copy link
Contributor Author

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented Oct 27, 2019

📌 Commit b26ddb8 has been approved by petrochenkov

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 27, 2019
Centril added a commit to Centril/rust that referenced this pull request Oct 27, 2019
Point at local similarly named element and tweak references to variants

Partially address rust-lang#65386.
@bors
Copy link
Contributor

bors commented Oct 28, 2019

⌛ Testing commit b26ddb8 with merge 8d78bf6...

bors added a commit that referenced this pull request Oct 28, 2019
Point at local similarly named element and tweak references to variants

Partially address #65386.
@bors
Copy link
Contributor

bors commented Oct 28, 2019

☀️ Test successful - checks-azure
Approved by: petrochenkov
Pushing 8d78bf6 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 28, 2019
@bors bors merged commit b26ddb8 into rust-lang:master Oct 28, 2019
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #65421!

Tested on commit 8d78bf6.
Direct link to PR: #65421

💔 rls on windows: test-pass → test-fail (cc @Xanewok, @rust-lang/infra).
💔 rls on linux: test-pass → test-fail (cc @Xanewok, @rust-lang/infra).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Oct 28, 2019
Tested on commit rust-lang/rust@8d78bf6.
Direct link to PR: <rust-lang/rust#65421>

💔 rls on windows: test-pass → test-fail (cc @Xanewok, @rust-lang/infra).
💔 rls on linux: test-pass → test-fail (cc @Xanewok, @rust-lang/infra).
bors added a commit that referenced this pull request Jan 11, 2020
Point at the span for the definition of crate foreign ADTs

Follow up to #65421. Partially addresses #65386. Blocked on #53081.
@estebank estebank deleted the variants branch November 9, 2023 05:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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