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

Some errors only show up with cargo build not cargo check #70923

Closed
2 tasks
oli-obk opened this issue Apr 8, 2020 · 7 comments
Closed
2 tasks

Some errors only show up with cargo build not cargo check #70923

oli-obk opened this issue Apr 8, 2020 · 7 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Apr 8, 2020

This is partially due to the fact that our analysis query

fn analysis(tcx: TyCtxt<'_>, cnum: CrateNum) -> Result<()> {
does not invoke the optimized_mir query on all items in the current crate, and also because we don't run monomorphization when building bin crates.

  • run optimized_mir query in analysis
    • this may cause slowdowns for cargo check because we'd be doing optimizations now
    • All const propagation lints are emitted in optimized_mir, but we may add more in the future.
  • run monomorphization where possible.
    • Not sure how useful this is, it will only give us additional lints in very specific situations. (e.g. when we subtract something form an associated constant, we may get an overflow error depending on the value of the constant).
    • This will also be significantly more expensive than just running optimized_mir

cc @Centril

@oli-obk oli-obk added the C-bug Category: This is a bug. label Apr 8, 2020
@jonas-schievink jonas-schievink added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 8, 2020
@RalfJung
Copy link
Member

RalfJung commented Apr 8, 2020

Or maybe it is a bad idea to emit lints during optimization, and we should move the lints from const-prop to elsewhere?

@jonas-schievink jonas-schievink added C-enhancement Category: An issue proposing an enhancement or a PR with one. and removed C-bug Category: This is a bug. labels Apr 8, 2020
@ehuss
Copy link
Contributor

ehuss commented Apr 8, 2020

This gets reported occasionally, like #49292 and #61330.

@oli-obk
Copy link
Contributor Author

oli-obk commented Apr 8, 2020

the reason the lints are part of const prop is to ensure we don't do the work twice, once for linting and once for the const prop optimization. I'm not sure how we could do that differently.

Also I was envisioning adding more lints to the optimization pipeline instead of the other way around.

@RalfJung
Copy link
Member

Do we have a concrete example / test case demonstrating this?

@mati865

This comment has been minimized.

@RalfJung
Copy link
Member

@ehuss has an example of this: it passes with cargo check, but fails cargo build, latest nightly rustc 1.45.0-nightly (fa51f810e 2020-04-29).

fn main() {
    let a: u8 = 42;
    let b = ((a | 0x0F) - 1) >> 31 > 0;
    println!("{}", b);
}

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=4998a76829fb44657cd8ed814fb2a191

@RalfJung
Copy link
Member

Closing as a duplicate of #49292 (for the diagnostics emitted in MIR passes) and #99682 (for errors during monomorphization).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. 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