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

Diagnostics in MIR passes (arithmetic_overflow, unconditional_panic) are not caught with cargo check #49292

Open
ishitatsuyuki opened this issue Mar 23, 2018 · 5 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ishitatsuyuki
Copy link
Contributor

I just tried to ran the compile-fail suite without trans, and these tests failed:

failures:
    [compile-fail] compile-fail/array_const_index-0.rs
    [compile-fail] compile-fail/array_const_index-1.rs
    [compile-fail] compile-fail/asm-src-loc.rs
    [compile-fail] compile-fail/bad-intrinsic-monomorphization.rs
    [compile-fail] compile-fail/cdylib-deps-must-be-static.rs
    [compile-fail] compile-fail/const-err-early.rs
    [compile-fail] compile-fail/const-err-multi.rs
    [compile-fail] compile-fail/const-err.rs
    [compile-fail] compile-fail/const-err2.rs
    [compile-fail] compile-fail/const-err3.rs
    [compile-fail] compile-fail/const-eval-overflow2.rs
    [compile-fail] compile-fail/const-eval-overflow2b.rs
    [compile-fail] compile-fail/const-eval-overflow2c.rs
    [compile-fail] compile-fail/const-slice-oob.rs
    [compile-fail] compile-fail/dupe-symbols-1.rs
    [compile-fail] compile-fail/dupe-symbols-2.rs
    [compile-fail] compile-fail/dupe-symbols-3.rs
    [compile-fail] compile-fail/dupe-symbols-4.rs
    [compile-fail] compile-fail/dupe-symbols-5.rs
    [compile-fail] compile-fail/dupe-symbols-6.rs
    [compile-fail] compile-fail/dupe-symbols-7.rs
    [compile-fail] compile-fail/huge-array.rs
    [compile-fail] compile-fail/huge-enum.rs
    [compile-fail] compile-fail/huge-struct.rs
    [compile-fail] compile-fail/impl-trait/infinite-impl-trait-issue-38064.rs
    [compile-fail] compile-fail/infinite-instantiation.rs
    [compile-fail] compile-fail/issue-10755.rs
    [compile-fail] compile-fail/issue-11154.rs
    [compile-fail] compile-fail/issue-15919.rs
    [compile-fail] compile-fail/issue-17913.rs
    [compile-fail] compile-fail/issue-22638.rs
    [compile-fail] compile-fail/issue-26548.rs
    [compile-fail] compile-fail/issue-44578.rs
    [compile-fail] compile-fail/issue-8460-const.rs
    [compile-fail] compile-fail/issue-8727.rs
    [compile-fail] compile-fail/linkage2.rs
    [compile-fail] compile-fail/linkage3.rs
    [compile-fail] compile-fail/lint-exceeding-bitshifts.rs
    [compile-fail] compile-fail/lint-exceeding-bitshifts2.rs
    [compile-fail] compile-fail/nolink-with-link-args.rs
    [compile-fail] compile-fail/non-interger-atomic.rs
    [compile-fail] compile-fail/panic-runtime/abort-link-to-unwind-dylib.rs
    [compile-fail] compile-fail/panic-runtime/libtest-unwinds.rs
    [compile-fail] compile-fail/panic-runtime/transitive-link-a-bunch.rs
    [compile-fail] compile-fail/panic-runtime/two-panic-runtimes.rs
    [compile-fail] compile-fail/panic-runtime/want-abort-got-unwind.rs
    [compile-fail] compile-fail/panic-runtime/want-abort-got-unwind2.rs
    [compile-fail] compile-fail/panic-runtime/want-unwind-got-abort.rs
    [compile-fail] compile-fail/panic-runtime/want-unwind-got-abort2.rs
    [compile-fail] compile-fail/recursion.rs
    [compile-fail] compile-fail/required-lang-item.rs
    [compile-fail] compile-fail/rmeta_lib.rs
    [compile-fail] compile-fail/simd-intrinsic-generic-arithmetic.rs
    [compile-fail] compile-fail/simd-intrinsic-generic-cast.rs
    [compile-fail] compile-fail/simd-intrinsic-generic-comparison.rs
    [compile-fail] compile-fail/simd-intrinsic-generic-elements.rs
    [compile-fail] compile-fail/simd-intrinsic-generic-reduction.rs
    [compile-fail] compile-fail/simd-type-generic-monomorphisation.rs
    [compile-fail] compile-fail/symbol-names/basic.rs
    [compile-fail] compile-fail/symbol-names/impl1.rs
    [compile-fail] compile-fail/type_length_limit.rs

All of those happens somewhere inside librustc_mir, most of them being monomorphization. This corresponds to translation item collection pass, which takes nontrivial amount of time.

Though, we should to try to evaluate things like struct layout, symbols even when we're doing check to catch all kind of errors.

@ishitatsuyuki ishitatsuyuki added the A-diagnostics Area: Messages for errors, warnings, and lints label Mar 23, 2018
@oli-obk oli-obk added the C-bug Category: This is a bug. label Mar 23, 2018
@cuviper cuviper added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 24, 2018
@gnzlbg
Copy link
Contributor

gnzlbg commented Mar 28, 2018

Some / all of the simd failures would need to move typeck from librustc_typeck to MIR typeck to avoid monomorphization time errors. This is something that should be done anyways and AFAICT there is no tracking issue for it.

@RalfJung
Copy link
Member

Here's a current example of this:

fn main() {
    let _val = 14u8 + 255;
}

The "overflow" diagnostic does not trigger with --emit=metadata.

I think the fix here would be to move this somewhere that is actually run during check:

&Lint(const_prop_lint::ConstProp),

I'm a bit surprised that doesn't already happen... I assume we do call mir_drops_elaborated_and_const_checked since that's where mir_borrowck is called, but then from there we should be calling run_analysis_to_runtime_passes which calls run_runtime_lowering_passes which runs the const_prop_link pass. I guess I am not understanding the control flow of our MIR pass manager -- as usual, that thing has always been messy.^^

@RalfJung
Copy link
Member

It would probably make sense to distinguish between "just produce the rmeta file as fast as possible" (check on dependencies) and "check the current crate (but fast)". It makes little sense to run the MIR lint passes on dependencies when lints are hidden anyway... but on the current crate I am working on I'd probably like to see these lints.

@RalfJung
Copy link
Member

Besides const_prop_lint, another pass that can emit errors is lower_intrinsics.

@rust-lang/wg-mir-opt if you know any other MIR passes that can emit errors or lints, please post them here. Also in the interest of keeping the differences between check and build as small as possible, it'd be good to avoid adding more such passes. :)

@oli-obk
Copy link
Contributor

oli-obk commented Aug 24, 2023

We can move the lower_intrinsics diagnostics to mir typeck.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Sep 7, 2023
…henkov

Don't report any errors in `lower_intrinsics`.

Intrinsics should have been type checked earlier.

This is part of moving all mir-opt diagnostics early enough so that they are reliably emitted even in check builds: rust-lang#49292 (comment)
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 7, 2023
…nkov

Don't report any errors in `lower_intrinsics`.

Intrinsics should have been type checked earlier.

This is part of moving all mir-opt diagnostics early enough so that they are reliably emitted even in check builds: rust-lang#49292 (comment)
@RalfJung RalfJung changed the title Diagnostics in MIR passes are not caught with cargo check Diagnostics in MIR passes (arithmetic_overflow, unconditional_panic) are not caught with cargo check Mar 18, 2024
mkeeter added a commit to mkeeter/fidget that referenced this issue Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants