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

Enable "jump to def" feature on patterns #134216

Merged
merged 4 commits into from
Jan 14, 2025

Conversation

GuillaumeGomez
Copy link
Member

Part of #89095.

Pattern (as in "patterns in pattern matching") were not handled by the feature, it's now added.

It all started when I realized that prelude values like Some or Err were not getting a link generated either (added support for it in the first commit).

r? @fmease

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Dec 12, 2024
@@ -189,6 +189,27 @@ impl SpanMapVisitor<'_> {
self.matches.insert(span, link);
}
}

fn handle_pat(&mut self, p: &Pat<'_>) {
Copy link
Member

@fmease fmease Dec 12, 2024

Choose a reason for hiding this comment

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

This function very closely mirrors intravisit::walk_pat. The only difference is the QPath handling. The more general and cleaner solution would be to explicitly implement visit_qself. This would automatically handle paths in expression, pattern and type contexts.

For example, rn we don't linkify Self::Var (ExprKind::Path(qpath)):

impl Trait for Ty { pub fn f() { let _ = Self::Var; } }
pub enum Ty { Var }
pub trait Trait { fn f(); }

Moreover, I presume we don't linkify Self::Assoc in impl ... for ... { type Ty = Self::Assoc; } (TyKind::Path(qpath)) but I can't check, I'm commuting atm.

However, this would require this "maybe_typeck_results/qpath_res " setup I mentioned on Zulip. Furthermore, it would support fully-qualified paths <Ty as TraitRef>::assoc which we don't on master.

Anyway, if you don't want to do that in this PR, I can send a follup-up PR that impls that.

My 2¢ – won't say more rn, I'm still commuting lol.

Copy link
Member Author

Choose a reason for hiding this comment

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

It was not in hurry for review you know. 😅 (But thanks!)

So do you want me to implement visit_qself in this PR or in a follow-up?

Copy link
Member

Choose a reason for hiding this comment

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

Let me get back home, then I'm going to answer that xD

@fmease
Copy link
Member

fmease commented Jan 14, 2025

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jan 14, 2025

📌 Commit 12a12c9 has been approved by fmease

It is now in the queue for this repository.

@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-review Status: Awaiting review from the assignee but also interested parties. labels Jan 14, 2025
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jan 14, 2025
…=fmease

Enable "jump to def" feature on patterns

Part of rust-lang#89095.

Pattern (as in "patterns in pattern matching") were not handled by the feature, it's now added.

It all started when I realized that prelude values like `Some` or `Err` were not getting a link generated either (added support for it in the first commit).

r? `@fmease`
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 14, 2025
…llaumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#134216 (Enable "jump to def" feature on patterns)
 - rust-lang#134353 (Treat safe target_feature functions as unsafe by default [less invasive variant])
 - rust-lang#134880 (Made `Path::name` only have item name rather than full name)
 - rust-lang#135466 (Leak check in `impossible_predicates` to avoid monomorphizing impossible instances)
 - rust-lang#135476 (Remove remnant of asmjs)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 14, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#134216 (Enable "jump to def" feature on patterns)
 - rust-lang#134880 (Made `Path::name` only have item name rather than full name)
 - rust-lang#135466 (Leak check in `impossible_predicates` to avoid monomorphizing impossible instances)
 - rust-lang#135476 (Remove remnant of asmjs)
 - rust-lang#135479 (mir borrowck: cleanup late-bound region handling)
 - rust-lang#135493 (Fix legacy symbol mangling of closures)
 - rust-lang#135495 (Add missing closing backtick in commit hook message 🐸)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit aeadee0 into rust-lang:master Jan 14, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Jan 14, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 14, 2025
Rollup merge of rust-lang#134216 - GuillaumeGomez:jump-to-def-pats, r=fmease

Enable "jump to def" feature on patterns

Part of rust-lang#89095.

Pattern (as in "patterns in pattern matching") were not handled by the feature, it's now added.

It all started when I realized that prelude values like `Some` or `Err` were not getting a link generated either (added support for it in the first commit).

r? ``@fmease``
@GuillaumeGomez GuillaumeGomez deleted the jump-to-def-pats branch January 15, 2025 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants