Don't ICE if TAIT-defining fn contains a closure with _ in return type
#119975
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @TaKO8Ki (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
compiler-errors
requested changes
Jan 14, 2024
| } | ||
| // Function items with `_` in their return type already emit an error, skip any | ||
| // "non-defining use" errors for them. | ||
| if let Some(hir_sig) = self.tcx.hir_node_by_def_id(item_def_id).fn_sig() |
There was a problem hiding this comment.
Can you change this to manually match on node kind, and only look for Node::Item/Node::TraitItem/Node::ImplItem/Node::ForeignItem?
The difference between when Node::fn_sig and Node::fn_decl return Some(_) is extremely subtle. It took me quite some time to even realize that's what changed in this PR.
There was a problem hiding this comment.
For Node::ForeignItem this isn't necessary because we already skip them here:
I've added a test to ensure that is the case and we don't emit any extra errors for them.
Without Node::ForeignItem, the manual match is now just Node::fn_sig manually inlined and I don't think that's a lot better. The real problem here is the ambigous naming of FnSig vs FnDecl, where FnSig = FnDecl + FnHeader and FnHeader = effects (const/aync/unsafe) + call abi.
There was a problem hiding this comment.
What about renaming it to fn_item_sig or fn_sig_of_item or something? Something that distinguishes that it's grabbing just the signature of an item.
There was a problem hiding this comment.
For fn_item_sig I'd expect it to also include foreign function items, which fn_sig does not.
There was a problem hiding this comment.
I've updated the comment regarding fn_sig vs fn_decl and added a debug_assert! to hopefully make it more clear what's happening here.
And I'd rather not bikeshed a new name for fn_sig in this PR, so with that,
@rustbot review
For the record, here is the manual match version, which is literally just inlined fn_sig: 79b8fa5
There was a problem hiding this comment.
I prefer the manual match version, but whatever
|
@rustbot author r=me after that tho |
rustbot
added
S-waiting-on-author
lukas-code
force-pushed
the
inferring-return-types-and-opaque-types-do-mix-sometimes
branch
from
3fccae4 to
669f847
Compare
lukas-code
force-pushed
the
inferring-return-types-and-opaque-types-do-mix-sometimes
branch
from
669f847 to
22833c1
Compare
rustbot
added
S-waiting-on-review
|
@bors r+ |
bors
added
S-waiting-on-bors
|
@bors rollup |
compiler-errors
added a commit
to compiler-errors/rust
that referenced
this pull request
Jan 17, 2024
…and-opaque-types-do-mix-sometimes, r=compiler-errors Don't ICE if TAIT-defining fn contains a closure with `_` in return type The `delay_span_bug` got added in rust-lang@0e82aae to reduce the amount of errors emitted for functions that have `_` in their return type, because inference doesn't apply to function items. But this logic shouldn't apply to closures, because their return types *can* be inferred. Fixes rust-lang#119916.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 17, 2024
…mpiler-errors Rollup of 9 pull requests Successful merges: - rust-lang#115291 (Save liveness results for DestinationPropagation) - rust-lang#119651 (proc_macro: Add Literal::c_string constructor) - rust-lang#119855 (Enable Static Builds for FreeBSD) - rust-lang#119955 (Modify GenericArg and Term structs to use strict provenance rules) - rust-lang#119975 (Don't ICE if TAIT-defining fn contains a closure with `_` in return type) - rust-lang#119984 (Change return type of unstable `Waker::noop()` from `Waker` to `&Waker`.) - rust-lang#120001 (Consistently unset RUSTC_BOOTSTRAP when compiling bootstrap) - rust-lang#120020 (Gracefully handle missing typeck information if typeck errored) - rust-lang#120032 (Fix `rustc_abi` build on stable) r? `@ghost` `@rustbot` modify labels: rollup
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Jan 17, 2024
…and-opaque-types-do-mix-sometimes, r=compiler-errors Don't ICE if TAIT-defining fn contains a closure with `_` in return type The `delay_span_bug` got added in rust-lang@0e82aae to reduce the amount of errors emitted for functions that have `_` in their return type, because inference doesn't apply to function items. But this logic shouldn't apply to closures, because their return types *can* be inferred. Fixes rust-lang#119916.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 17, 2024
…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#115291 (Save liveness results for DestinationPropagation) - rust-lang#119855 (Enable Static Builds for FreeBSD) - rust-lang#119975 (Don't ICE if TAIT-defining fn contains a closure with `_` in return type) - rust-lang#119984 (Change return type of unstable `Waker::noop()` from `Waker` to `&Waker`.) - rust-lang#120001 (Consistently unset RUSTC_BOOTSTRAP when compiling bootstrap) - rust-lang#120020 (Gracefully handle missing typeck information if typeck errored) - rust-lang#120031 (Construct closure type eagerly) - rust-lang#120032 (Fix `rustc_abi` build on stable) - rust-lang#120039 (pat_analysis: Don't rely on contiguous `VariantId`s outside of rustc) - rust-lang#120044 (Fix typo in comments (in_place_collect)) - rust-lang#120056 (Use FnOnceOutput instead of FnOnce where expected) r? `@ghost` `@rustbot` modify labels: rollup
compiler-errors
added a commit
to compiler-errors/rust
that referenced
this pull request
Jan 17, 2024
…and-opaque-types-do-mix-sometimes, r=compiler-errors Don't ICE if TAIT-defining fn contains a closure with `_` in return type The `delay_span_bug` got added in rust-lang@0e82aae to reduce the amount of errors emitted for functions that have `_` in their return type, because inference doesn't apply to function items. But this logic shouldn't apply to closures, because their return types *can* be inferred. Fixes rust-lang#119916.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 17, 2024
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#115291 (Save liveness results for DestinationPropagation) - rust-lang#119855 (Enable Static Builds for FreeBSD) - rust-lang#119975 (Don't ICE if TAIT-defining fn contains a closure with `_` in return type) - rust-lang#120001 (Consistently unset RUSTC_BOOTSTRAP when compiling bootstrap) - rust-lang#120020 (Gracefully handle missing typeck information if typeck errored) - rust-lang#120031 (Construct closure type eagerly) - rust-lang#120032 (Fix `rustc_abi` build on stable) - rust-lang#120039 (pat_analysis: Don't rely on contiguous `VariantId`s outside of rustc) - rust-lang#120044 (Fix typo in comments (in_place_collect)) - rust-lang#120056 (Use FnOnceOutput instead of FnOnce where expected) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 18, 2024
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#115291 (Save liveness results for DestinationPropagation) - rust-lang#119855 (Enable Static Builds for FreeBSD) - rust-lang#119975 (Don't ICE if TAIT-defining fn contains a closure with `_` in return type) - rust-lang#120001 (Consistently unset RUSTC_BOOTSTRAP when compiling bootstrap) - rust-lang#120020 (Gracefully handle missing typeck information if typeck errored) - rust-lang#120031 (Construct closure type eagerly) - rust-lang#120032 (Fix `rustc_abi` build on stable) - rust-lang#120039 (pat_analysis: Don't rely on contiguous `VariantId`s outside of rustc) - rust-lang#120044 (Fix typo in comments (in_place_collect)) - rust-lang#120056 (Use FnOnceOutput instead of FnOnce where expected) r? `@ghost` `@rustbot` modify labels: rollup
lukas-code
deleted the
inferring-return-types-and-opaque-types-do-mix-sometimes
branch
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 18, 2024
Rollup merge of rust-lang#119975 - lukas-code:inferring-return-types-and-opaque-types-do-mix-sometimes, r=compiler-errors Don't ICE if TAIT-defining fn contains a closure with `_` in return type The `delay_span_bug` got added in rust-lang@0e82aae to reduce the amount of errors emitted for functions that have `_` in their return type, because inference doesn't apply to function items. But this logic shouldn't apply to closures, because their return types *can* be inferred. Fixes rust-lang#119916.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
delay_span_buggot added in 0e82aae to reduce the amount of errors emitted for functions that have_in their return type, because inference doesn't apply to function items. But this logic shouldn't apply to closures, because their return types can be inferred.Fixes #119916.