Add tests and docs for `#[derive(GenericTypeVisitable)]` by ada4a · Pull Request #161806 · rust-lang/rust

ada4a · 2026-08-26T10:41:05Z

..given the added complexity from the newly-added bounds attribute

Follow-up to #160914

More details in individual commits.

cc @JonathanBrouwer (you might want to take over the review of this since you have some context already.. but as you wish)
cc @ChayimFriedman2

rustbot · 2026-08-26T10:41:08Z

rustc-dev-guide is developed in its own repository. If possible, consider making this change to rust-lang/rustc-dev-guide instead.

cc @BoxyUwU, @tshepang

rustbot · 2026-08-26T10:41:11Z

r? @mejrs

rustbot has assigned @mejrs.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 75 candidates
Random selection from 19 candidates

JonathanBrouwer · 2026-08-26T10:52:30Z

r? me
Yeah I can take this, feel free to just assign to me in the future :3

rustbot · 2026-08-26T12:13:29Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

JonathanBrouwer · 2026-08-26T12:13:44Z

Other than that this looks good to me, thanks!

JonathanBrouwer · 2026-09-07T20:04:28Z

+
+unsafe impl<V> GenericTypeVisitable<V> for Incrementer {
+    fn generic_visit_with(&self, _visitor: &mut V) {
+        COUNT.fetch_add(1, Ordering::AcqRel);


nit: Ordering::Relaxed should be fine here and below.
This ofc doesn't matter for performance because this is merely a test, but imo using anything other than Relaxed should only be done if there is a reason for it, as otherwise it might be unnecessarily confusing to future readers

View changes since the review

I was just being extra cautious.. atomics scary. But given that everything here is happening in one thread, Relaxed should indeed be fine.

Even if there were multiple threads, we don't rely on the ordering of reads/writes of COUNT relative to other atomic operations, so Relaxed should be fine

Two threads fetch_add-ing the value of COUNT at the same time with Ordering::Relaxed would race though, no?

JonathanBrouwer

other than that this looks reasonable, thanks so much for putting effort into making the test :3

View changes since this review

JonathanBrouwer · 2026-09-07T20:08:55Z

(also please rebase)
@rustbot author

I ended up deciding not to add docs about `bounds` as it seems like a relatively minor feature of the derive, and there are docs at [1]. [1]: https://github.com/rust-lang/rust/blob/3ffb26fbf5bf232cf59e314e75ea325973f4f583/compiler/rustc_type_ir_macros/src/lib.rs#L21-L55

Just specifying `T: GenericTypeVisitable` doesn't work, as the trait has a generic: `V`, the visitor. `T: GenericTypeVisitable<__V>` is what actually works, as `__V` is the generic added to the impl generated by the derive macro. We discussed[1] different ways of making this nicer, but settled on not doing anything, as we don't expect people to need to specify any actual bounds. [1]: https://rust-lang.zulipchat.com/#narrow/channel/185405-t-compiler.2Frust-analyzer/topic/Updating.20next-solver/near/618331780 and below

rustbot · 2026-09-07T20:20:06Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

ada4a · 2026-09-07T20:23:18Z

(forgot to actually git add the requested changes before pushing.. this is what happens when you return to Git after a while 😅)

ada4a · 2026-09-07T20:25:55Z

@rustbot ready

(it never feels right to post this before the CI is green, but hey, it passes on my machine so it should be fine right :p )

JonathanBrouwer

@bors r+ rollup

View changes since this review

rust-bors · 2026-09-08T08:44:02Z

📌 Commit ada4f67 has been approved by JonathanBrouwer

It is now in the queue for this repository.

@JonathanBrouwer

…nBrouwer Add tests and docs for `#[derive(GenericTypeVisitable)]` ..given the added complexity from the newly-added `bounds` attribute Follow-up to rust-lang#160914 More details in individual commits. cc @JonathanBrouwer (you might want to take over the review of this since you have some context already.. but as you wish) cc @ChayimFriedman2

@JonathanBrouwer

…nBrouwer Add tests and docs for `#[derive(GenericTypeVisitable)]` ..given the added complexity from the newly-added `bounds` attribute Follow-up to rust-lang#160914 More details in individual commits. cc @JonathanBrouwer (you might want to take over the review of this since you have some context already.. but as you wish) cc @ChayimFriedman2

…uwer Rollup of 8 pull requests Successful merges: - #160505 (delegation: supporting inherent impls) - #160651 (mir: validate `Move` call arguments are locals or box derefs) - #161806 (Add tests and docs for `#[derive(GenericTypeVisitable)]`) - #161912 (run `extern "tail"` with `byval` argument test) - #162435 (windows-gnu: document libgcc requirement) - #162439 (Update books) - #162459 (docs(time): replace "method" with "function") - #162465 (Fix my duplicate thanks entry)

@JonathanBrouwer

…nBrouwer Add tests and docs for `#[derive(GenericTypeVisitable)]` ..given the added complexity from the newly-added `bounds` attribute Follow-up to rust-lang#160914 More details in individual commits. cc @JonathanBrouwer (you might want to take over the review of this since you have some context already.. but as you wish) cc @ChayimFriedman2

…uwer Rollup of 10 pull requests Successful merges: - #162470 (Subtree sync for rustc_codegen_cranelift) - #160505 (delegation: supporting inherent impls) - #160651 (mir: validate `Move` call arguments are locals or box derefs) - #161806 (Add tests and docs for `#[derive(GenericTypeVisitable)]`) - #161912 (run `extern "tail"` with `byval` argument test) - #162435 (windows-gnu: document libgcc requirement) - #162439 (Update books) - #162451 (Add regression test for item-local diagnostic attribute lint levels) - #162459 (docs(time): replace "method" with "function") - #162465 (Fix my duplicate thanks entry)

@JonathanBrouwer

…nBrouwer Add tests and docs for `#[derive(GenericTypeVisitable)]` ..given the added complexity from the newly-added `bounds` attribute Follow-up to rust-lang#160914 More details in individual commits. cc @JonathanBrouwer (you might want to take over the review of this since you have some context already.. but as you wish) cc @ChayimFriedman2

…uwer Rollup of 9 pull requests Successful merges: - #160505 (delegation: supporting inherent impls) - #160651 (mir: validate `Move` call arguments are locals or box derefs) - #161806 (Add tests and docs for `#[derive(GenericTypeVisitable)]`) - #161912 (run `extern "tail"` with `byval` argument test) - #162435 (windows-gnu: document libgcc requirement) - #162439 (Update books) - #162451 (Add regression test for item-local diagnostic attribute lint levels) - #162459 (docs(time): replace "method" with "function") - #162465 (Fix my duplicate thanks entry)

rust-bors · 2026-09-08T12:24:05Z

⌛ Testing commit ada4f67 with merge a366d35...

Workflow: https://github.com/rust-lang/rust/actions/runs/34225864268

@JonathanBrouwer

Add tests and docs for `#[derive(GenericTypeVisitable)]` ..given the added complexity from the newly-added `bounds` attribute Follow-up to #160914 More details in individual commits. cc @JonathanBrouwer (you might want to take over the review of this since you have some context already.. but as you wish) cc @ChayimFriedman2

JonathanBrouwer · 2026-09-08T12:25:44Z

@bors yield
Yielding to enclosing rollup

rust-bors · 2026-09-08T12:25:48Z

Auto build was cancelled. Cancelled workflows:

https://github.com/rust-lang/rust/actions/runs/34225864268

The next pull request likely to be tested is #162474.

…uwer Rollup of 9 pull requests Successful merges: - #160505 (delegation: supporting inherent impls) - #160651 (mir: validate `Move` call arguments are locals or box derefs) - #161806 (Add tests and docs for `#[derive(GenericTypeVisitable)]`) - #161912 (run `extern "tail"` with `byval` argument test) - #162435 (windows-gnu: document libgcc requirement) - #162439 (Update books) - #162451 (Add regression test for item-local diagnostic attribute lint levels) - #162459 (docs(time): replace "method" with "function") - #162465 (Fix my duplicate thanks entry)

@JonathanBrouwer

…nBrouwer Add tests and docs for `#[derive(GenericTypeVisitable)]` ..given the added complexity from the newly-added `bounds` attribute Follow-up to rust-lang#160914 More details in individual commits. cc @JonathanBrouwer (you might want to take over the review of this since you have some context already.. but as you wish) cc @ChayimFriedman2

…uwer Rollup of 12 pull requests Successful merges: - #162462 (Fix unsoundness in `VecDeque::from_iter(vec::IntoIter)`) - #162468 (mir_build: Rename `lower_let_expr` to `lower_fallible_let`, and clarify) - #161806 (Add tests and docs for `#[derive(GenericTypeVisitable)]`) - #161912 (run `extern "tail"` with `byval` argument test) - #161932 (Only check `#[allow(dead_code)]` on the trait) - #162236 (Call renumberMetadataForAssembly() in LLVMRustPrintModule()) - #162435 (windows-gnu: document libgcc requirement) - #162439 (Update books) - #162451 (Add regression test for item-local diagnostic attribute lint levels) - #162455 (Update codegen-llvm/inline-debuginfo.rs for distinct DILocation) - #162459 (docs(time): replace "method" with "function") - #162465 (Fix my duplicate thanks entry)

rustbot added A-rustc-dev-guide Area: rustc-dev-guide S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 26, 2026

rustbot assigned mejrs Aug 26, 2026

rustbot assigned JonathanBrouwer and unassigned mejrs Aug 26, 2026

JonathanBrouwer reviewed Aug 26, 2026

View reviewed changes

Comment thread tests/ui-fulldeps/derive-generic-type-visitable.rs

JonathanBrouwer reviewed Aug 26, 2026

View reviewed changes

Comment thread tests/ui-fulldeps/derive-generic-type-visitable.rs Outdated

JonathanBrouwer requested changes Aug 26, 2026

View reviewed changes

rustbot 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 Aug 26, 2026

This comment has been minimized.

Sign in to view

ada4a force-pushed the push-olrruxoktqnl branch from dbcc3e0 to a570f6b Compare September 7, 2026 19:48

JonathanBrouwer reviewed Sep 7, 2026

View reviewed changes

JonathanBrouwer approved these changes Sep 7, 2026

View reviewed changes

ada4a added 3 commits September 7, 2026 22:10

misc typo fixes

fa2850e

ada4a force-pushed the push-olrruxoktqnl branch from a570f6b to 12dd807 Compare September 7, 2026 20:20

add tests for #[derive(GenericTypeVisitable)] and bounds

ada4f67

ada4a force-pushed the push-olrruxoktqnl branch from 12dd807 to ada4f67 Compare September 7, 2026 20:22

rustbot 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 Sep 7, 2026

JonathanBrouwer approved these changes Sep 8, 2026

View reviewed changes

rust-bors Bot 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 Sep 8, 2026

JonathanBrouwer mentioned this pull request Sep 8, 2026

Rollup of 6 pull requests #162464

Closed

JonathanBrouwer mentioned this pull request Sep 8, 2026

Rollup of 8 pull requests #162466

Closed

JonathanBrouwer mentioned this pull request Sep 8, 2026

Rollup of 10 pull requests #162472

Closed

JonathanBrouwer mentioned this pull request Sep 8, 2026

Rollup of 9 pull requests #162474

Closed

JonathanBrouwer mentioned this pull request Sep 8, 2026

Rollup of 12 pull requests #162477

Open

Uh oh!

Uh oh!

Conversation

ada4a commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Aug 26, 2026

Uh oh!

rustbot commented Aug 26, 2026

Uh oh!

JonathanBrouwer commented Aug 26, 2026

Uh oh!

Uh oh!

Uh oh!

rustbot commented Aug 26, 2026

Uh oh!

JonathanBrouwer commented Aug 26, 2026

Uh oh!

This comment has been minimized.

JonathanBrouwer Sep 7, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ada4a Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

ada4a Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer commented Sep 7, 2026

Uh oh!

rustbot commented Sep 7, 2026

Uh oh!

ada4a commented Sep 7, 2026

Uh oh!

ada4a commented Sep 7, 2026

Uh oh!

JonathanBrouwer left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rust-bors Bot commented Sep 8, 2026

Uh oh!

rust-bors Bot commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JonathanBrouwer commented Sep 8, 2026

Uh oh!

rust-bors Bot commented Sep 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ada4a commented Aug 26, 2026 •

edited

Loading

JonathanBrouwer Sep 7, 2026 •

edited by rustbot

Loading

JonathanBrouwer left a comment •

edited by rustbot

Loading

JonathanBrouwer left a comment •

edited by rustbot

Loading

rust-bors Bot commented Sep 8, 2026 •

edited

Loading