-
Notifications
You must be signed in to change notification settings - Fork 13.6k
more strongly dissuade use of skip_binder
#144775
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
Conversation
78b4cd0
to
fc46354
Compare
/// Please read <https://rustc-dev-guide.rust-lang.org/ty_module/instantiating_binders.html> | ||
/// before using this function. It is usually better to discharge the binder using | ||
/// `no_bound_vars` or `instantiate_bound_regions` or something like that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider wrapping this and maybe its following paragraphs in a big warning block by surrounding it with <div class="warning">
</div>
(note: there needs to be a blank line between the HTML tags and the Markdown content or else the latter would be interpreted as HTML), similarly for the EarlyBinder
case.
These get rendered like so: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.TyCtxt.html#method.expand_free_alias_tys
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I feel I'd need to already mark the "Accessing generic args in the returned value is generally incorrect." as a warning, but that's part of the first line.
I personally don't know how I'd restructure this to add a warning block and would merge this as is
@bors r+ rollup I agree it'd be nice to have an actual warning here 🤔 but not sure how to structure this to do that nicely. can always do that in a follow up PR |
Rollup of 8 pull requests Successful merges: - #139451 (Add `target_env = "macabi"` and `target_env = "sim"`) - #144039 (Use `tcx.short_string()` in more diagnostics) - #144192 (atomicrmw on pointers: move integer-pointer cast hacks into backend) - #144545 (In rustc_pattern_analysis, put `true` witnesses before `false` witnesses) - #144579 (Implement declarative (`macro_rules!`) attribute macros (RFC 3697)) - #144649 (Account for bare tuples and `Pin` methods in field searching logic) - #144775 (more strongly dissuade use of `skip_binder`) - #144987 (Enable f16 and f128 on targets that were fixed in LLVM21) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #144775 - lcnr:skip_binder-comment, r=BoxyUwU more strongly dissuade use of `skip_binder` People unfortunately encounter `Binder` and `EarlyBinder` very early on when starting out. In these cases its often very easy to use `skip_binder` incorrectly. This makes it more explicit that it should generally not be used and points to the relevant `rustc-dev-guide` chapters. r? `@BoxyUwU`
Rollup of 8 pull requests Successful merges: - rust-lang/rust#139451 (Add `target_env = "macabi"` and `target_env = "sim"`) - rust-lang/rust#144039 (Use `tcx.short_string()` in more diagnostics) - rust-lang/rust#144192 (atomicrmw on pointers: move integer-pointer cast hacks into backend) - rust-lang/rust#144545 (In rustc_pattern_analysis, put `true` witnesses before `false` witnesses) - rust-lang/rust#144579 (Implement declarative (`macro_rules!`) attribute macros (RFC 3697)) - rust-lang/rust#144649 (Account for bare tuples and `Pin` methods in field searching logic) - rust-lang/rust#144775 (more strongly dissuade use of `skip_binder`) - rust-lang/rust#144987 (Enable f16 and f128 on targets that were fixed in LLVM21) r? `@ghost` `@rustbot` modify labels: rollup
People unfortunately encounter
Binder
andEarlyBinder
very early on when starting out. In these cases its often very easy to useskip_binder
incorrectly. This makes it more explicit that it should generally not be used and points to the relevantrustc-dev-guide
chapters.r? @BoxyUwU