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

Gracefully handle missing typeck information if typeck errored #120020

Merged
merged 2 commits into from
Jan 18, 2024

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jan 16, 2024

fixes #116893

I created some logs and the typeck of fn main is exactly the same, no matter whether the constant's body is what it is, or if it is replaced with panic!(). The latter will cause the ICE not to be emitted though. The reason for that is that we abort compilation if errors were emitted, but not if lint errors were emitted. This took me way too long to debug, and is another reason why I would have liked rust-lang/compiler-team#633

@rustbot
Copy link
Collaborator

rustbot commented Jan 16, 2024

r? @compiler-errors

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added 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 Jan 16, 2024
@Noratrieb
Copy link
Member

Noratrieb commented Jan 16, 2024

This has the problem that now, x will be marked as unused:

fn x() {}

fn main() {
    let s = [(); { loop {} }];

    x();
}
error: constant evaluation is taking a long time
 --> uwu.rs:4:20
  |
4 |     let s = [(); { loop {} }];
  |                    ^^^^^^^
  |
  = note: this lint makes sure the compiler doesn't get stuck due to infinite loops in const eval.
          If your compilation actually takes a long time, you can safely allow the lint.
help: the constant being evaluated
 --> uwu.rs:4:18
  |
4 |     let s = [(); { loop {} }];
  |                  ^^^^^^^^^^^
  = note: `#[deny(long_running_const_eval)]` on by default

warning: function `x` is never used
 --> uwu.rs:1:4
  |
1 | fn x() {}
  |    ^
  |
  = note: `#[warn(dead_code)]` on by default

error: aborting due to 1 previous error; 1 warning emitted

Because dead_code assumes dead until proven live. Would it make more sense to do it at a finer grained level instead of the body?

@oli-obk
Copy link
Contributor Author

oli-obk commented Jan 16, 2024

🤦 good point. Yea we can do it more fine-grained, but some ways to do that are effectively papering over some issues. I'll check if there is a reasonable way to do it

@oli-obk oli-obk force-pushed the long_const_eval_err_taint branch from 0787a5f to 3599c18 Compare January 16, 2024 10:52
@oli-obk oli-obk changed the title Skip dead code checks on items that failed typeck Gracefully handle missing typeck information if typeck errored Jan 16, 2024
@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jan 16, 2024

📌 Commit 3599c18 has been approved by compiler-errors

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 16, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 16, 2024
… r=compiler-errors

Gracefully handle missing typeck information if typeck errored

fixes rust-lang#116893

I created some logs and the typeck of `fn main` is exactly the same, no matter whether the constant's body is what it is, or if it is replaced with `panic!()`. The latter will cause the ICE not to be emitted though. The reason for that is that we abort compilation if *errors* were emitted, but not if *lint errors* were emitted. This took me way too long to debug, and is another reason why I would have liked rust-lang/compiler-team#633
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 16, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#119062 (Deny braced macro invocations in let-else)
 - rust-lang#119922 (Rework how diagnostic lints are stored.)
 - rust-lang#119978 (Move async closure parameters into the resultant closure's future eagerly)
 - rust-lang#119984 (Change return type of unstable `Waker::noop()` from `Waker` to `&Waker`.)
 - rust-lang#120020 (Gracefully handle missing typeck information if typeck errored)
 - rust-lang#120021 (don't store const var origins for known vars)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 16, 2024
… r=compiler-errors

Gracefully handle missing typeck information if typeck errored

fixes rust-lang#116893

I created some logs and the typeck of `fn main` is exactly the same, no matter whether the constant's body is what it is, or if it is replaced with `panic!()`. The latter will cause the ICE not to be emitted though. The reason for that is that we abort compilation if *errors* were emitted, but not if *lint errors* were emitted. This took me way too long to debug, and is another reason why I would have liked rust-lang/compiler-team#633
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 17, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#119855 (Enable Static Builds for FreeBSD)
 - rust-lang#119922 (Rework how diagnostic lints are stored.)
 - rust-lang#119978 (Move async closure parameters into the resultant closure's future eagerly)
 - rust-lang#119984 (Change return type of unstable `Waker::noop()` from `Waker` to `&Waker`.)
 - rust-lang#120020 (Gracefully handle missing typeck information if typeck errored)
 - rust-lang#120021 (don't store const var origins for known vars)
 - rust-lang#120032 (Fix `rustc_abi` build on stable)

r? `@ghost`
`@rustbot` modify labels: rollup
@compiler-errors
Copy link
Member

@bors rollup

compiler-errors added a commit to compiler-errors/rust that referenced this pull request Jan 17, 2024
… r=compiler-errors

Gracefully handle missing typeck information if typeck errored

fixes rust-lang#116893

I created some logs and the typeck of `fn main` is exactly the same, no matter whether the constant's body is what it is, or if it is replaced with `panic!()`. The latter will cause the ICE not to be emitted though. The reason for that is that we abort compilation if *errors* were emitted, but not if *lint errors* were emitted. This took me way too long to debug, and is another reason why I would have liked rust-lang/compiler-team#633
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
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 17, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#119855 (Enable Static Builds for FreeBSD)
 - rust-lang#119922 (Rework how diagnostic lints are stored.)
 - rust-lang#119978 (Move async closure parameters into the resultant closure's future eagerly)
 - rust-lang#119984 (Change return type of unstable `Waker::noop()` from `Waker` to `&Waker`.)
 - rust-lang#120020 (Gracefully handle missing typeck information if typeck errored)
 - rust-lang#120021 (don't store const var origins for known vars)
 - 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
… r=compiler-errors

Gracefully handle missing typeck information if typeck errored

fixes rust-lang#116893

I created some logs and the typeck of `fn main` is exactly the same, no matter whether the constant's body is what it is, or if it is replaced with `panic!()`. The latter will cause the ICE not to be emitted though. The reason for that is that we abort compilation if *errors* were emitted, but not if *lint errors* were emitted. This took me way too long to debug, and is another reason why I would have liked rust-lang/compiler-team#633
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
… r=compiler-errors

Gracefully handle missing typeck information if typeck errored

fixes rust-lang#116893

I created some logs and the typeck of `fn main` is exactly the same, no matter whether the constant's body is what it is, or if it is replaced with `panic!()`. The latter will cause the ICE not to be emitted though. The reason for that is that we abort compilation if *errors* were emitted, but not if *lint errors* were emitted. This took me way too long to debug, and is another reason why I would have liked rust-lang/compiler-team#633
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
@bors bors merged commit 6ca77ff into rust-lang:master Jan 18, 2024
11 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Jan 18, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 18, 2024
Rollup merge of rust-lang#120020 - oli-obk:long_const_eval_err_taint, r=compiler-errors

Gracefully handle missing typeck information if typeck errored

fixes rust-lang#116893

I created some logs and the typeck of `fn main` is exactly the same, no matter whether the constant's body is what it is, or if it is replaced with `panic!()`. The latter will cause the ICE not to be emitted though. The reason for that is that we abort compilation if *errors* were emitted, but not if *lint errors* were emitted. This took me way too long to debug, and is another reason why I would have liked rust-lang/compiler-team#633
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-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ice: no type-dependent def for method
5 participants