-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Do not call fn_sig on non-functions. #105201
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
r? @davidtwco (rustbot has picked a reviewer for you, use r? to override) |
Does this also fix #89271 ? |
Yes. |
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.
r=me comment addressed or not
// Do not call `fn_sig` on non-functions. | ||
if !matches!( | ||
self.tcx.def_kind(def_id), | ||
DefKind::Fn | DefKind::AssocFn | DefKind::Variant | DefKind::Ctor(..) |
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, what about closures? Side-note, there's DefKind::is_fn_like
, but it's slightly different that this case... Maybe worth checking use-cases and syncing them up.
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.
Wait, closures make no sense -- they have no where clauses, lol.
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.
Calling fn_sig
on closures and generators ICEs too.
@bors r+ rollup |
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#104199 (Keep track of the start of the argument block of a closure) - rust-lang#105050 (Remove useless borrows and derefs) - rust-lang#105153 (Create a hacky fail-fast mode that stops tests at the first failure) - rust-lang#105164 (Restore `use` suggestion for `dyn` method call requiring `Sized`) - rust-lang#105193 (Disable coverage instrumentation for naked functions) - rust-lang#105200 (Remove useless filter in unused extern crate check.) - rust-lang#105201 (Do not call fn_sig on non-functions.) - rust-lang#105208 (Add AmbiguityError for inconsistent resolution for an import) - rust-lang#105214 (update Miri) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
…fn-ptr-arg, r=cjgillot Point at args in associated const fn pointers Tiny follow-up to rust-lang#105201, not so sure it's worth it but 🤷 The UI test example is a bit more compelling when it's `GlUniformScalar::FACTORY` r? `@cjgillot`
…r=cjgillot Don't perform invalid checks in `codegen_attrs` The attributes `#[track_caller]` and `#[cmse_nonsecure_entry]` are only valid on functions. When validating one of these attributes, codegen_attrs previously called `fn_sig`, [which can only be used on functions](rust-lang#105201), on the item the attribute was attached to, assuming that the item was a function without checking. This led to [ICEs in situations where the attribute was incorrectly used on non-functions](rust-lang#105594). With this change, we skip calling `fn_sig` if the item the attribute is attached to must be a function but isn't, because `check_attr` will reject such cases without codegen_attrs's intervention. As a side note, some of the attributes in codegen_attrs are only valid on functions, but that property isn't actually checked. I'm planning to fix that in a follow up PR since it's a behavior change that will need to be validated rather than an obvious bugfix. Thankfully, all the attributes like that I've found so far are unstable. Fixes rust-lang#105594. r? `@cjgillot`
Don't perform invalid checks in `codegen_attrs` The attributes `#[track_caller]` and `#[cmse_nonsecure_entry]` are only valid on functions. When validating one of these attributes, codegen_attrs previously called `fn_sig`, [which can only be used on functions](rust-lang/rust#105201), on the item the attribute was attached to, assuming that the item was a function without checking. This led to [ICEs in situations where the attribute was incorrectly used on non-functions](rust-lang/rust#105594). With this change, we skip calling `fn_sig` if the item the attribute is attached to must be a function but isn't, because `check_attr` will reject such cases without codegen_attrs's intervention. As a side note, some of the attributes in codegen_attrs are only valid on functions, but that property isn't actually checked. I'm planning to fix that in a follow up PR since it's a behavior change that will need to be validated rather than an obvious bugfix. Thankfully, all the attributes like that I've found so far are unstable. Fixes #105594. r? `@cjgillot`
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#104199 (Keep track of the start of the argument block of a closure) - rust-lang#105050 (Remove useless borrows and derefs) - rust-lang#105153 (Create a hacky fail-fast mode that stops tests at the first failure) - rust-lang#105164 (Restore `use` suggestion for `dyn` method call requiring `Sized`) - rust-lang#105193 (Disable coverage instrumentation for naked functions) - rust-lang#105200 (Remove useless filter in unused extern crate check.) - rust-lang#105201 (Do not call fn_sig on non-functions.) - rust-lang#105208 (Add AmbiguityError for inconsistent resolution for an import) - rust-lang#105214 (update Miri) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Fixes #105040
Fixes #89271