codegen: assume constants cannot fail to evaluate

[RalfJung]

#80579 landed, so we can finally remove this old hack from codegen and instead assume that consts never fail to evaluate. :)

r? @oli-obk

[rust-log-analyzer]

The job x86_64-gnu-llvm-9 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

failures:

---- [ui] ui/consts/assoc_const_generic_impl.rs stdout ----

error: Error: expected failure status (Some(1)) but received status Some(101).
status: exit code: 101
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/consts/assoc_const_generic_impl.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zemit-future-incompat-report" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/consts/assoc_const_generic_impl" "-A" "unused" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/consts/assoc_const_generic_impl/auxiliary"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
warning: any use of this value will cause an error
   |
   |
LL |     const I_AM_ZERO_SIZED: ()  = [()][std::mem::size_of::<Self>()]; //~ WARN any use of this value
   |                                  |
   |                                  |
   |                                  index out of bounds: the length is 1 but the index is 4
note: the lint level is defined here
  --> /checkout/src/test/ui/consts/assoc_const_generic_impl.rs:3:9
   |
   |
LL | #![warn(const_err)]

error: erroneous constant encountered
  --> /checkout/src/test/ui/consts/assoc_const_generic_impl.rs:13:18
   |
   |
LL |         let () = Self::I_AM_ZERO_SIZED; //~ ERROR erroneous constant encountered


Basic Block in function '_ZN57_$LT$T$u20$as$u20$assoc_const_generic_impl..ZeroSized$GT$18requires_zero_size17h2735d8dd43a3946bE' does not have terminator!
label %2
in function _ZN57_$LT$T$u20$as$u20$assoc_const_generic_impl..ZeroSized$GT$18requires_zero_size17h2735d8dd43a3946bE
LLVM ERROR: Broken function found, compilation aborted!
------------------------------------------



---

Some tests failed in compiletest suite=ui mode=ui host=x86_64-unknown-linux-gnu target=x86_64-unknown-linux-gnu


command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--suite" "ui" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-9/bin/FileCheck" "--nodejs" "/usr/bin/node" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python3" "--lldb-python" "/usr/bin/python3" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "9.0.0" "--llvm-components" "aarch64 aarch64asmparser aarch64codegen aarch64desc aarch64disassembler aarch64info aarch64utils aggressiveinstcombine all all-targets amdgpu amdgpuasmparser amdgpucodegen amdgpudesc amdgpudisassembler amdgpuinfo amdgpuutils analysis arm armasmparser armcodegen armdesc armdisassembler arminfo armutils asmparser asmprinter avr avrasmparser avrcodegen avrdesc avrdisassembler avrinfo binaryformat bitreader bitstreamreader bitwriter bpf bpfasmparser bpfcodegen bpfdesc bpfdisassembler bpfinfo codegen core coroutines coverage debuginfocodeview debuginfodwarf debuginfogsym debuginfomsf debuginfopdb demangle dlltooldriver engine executionengine fuzzmutate globalisel hexagon hexagonasmparser hexagoncodegen hexagondesc hexagondisassembler hexagoninfo instcombine instrumentation interpreter ipo irreader jitlink lanai lanaiasmparser lanaicodegen lanaidesc lanaidisassembler lanaiinfo libdriver lineeditor linker lto mc mca mcdisassembler mcjit mcparser mips mipsasmparser mipscodegen mipsdesc mipsdisassembler mipsinfo mirparser msp430 msp430asmparser msp430codegen msp430desc msp430disassembler msp430info native nativecodegen nvptx nvptxcodegen nvptxdesc nvptxinfo objcarcopts object objectyaml option orcjit passes perfjitevents powerpc powerpcasmparser powerpccodegen powerpcdesc powerpcdisassembler powerpcinfo profiledata remarks riscv riscvasmparser riscvcodegen riscvdesc riscvdisassembler riscvinfo riscvutils runtimedyld scalaropts selectiondag sparc sparcasmparser sparccodegen sparcdesc sparcdisassembler sparcinfo support symbolize systemz systemzasmparser systemzcodegen systemzdesc systemzdisassembler systemzinfo tablegen target textapi transformutils vectorize webassembly webassemblyasmparser webassemblycodegen webassemblydesc webassemblydisassembler webassemblyinfo windowsmanifest x86 x86asmparser x86codegen x86desc x86disassembler x86info x86utils xcore xcorecodegen xcoredesc xcoredisassembler xcoreinfo xray" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"


failed to run: /checkout/obj/build/bootstrap/debug/bootstrap --stage 2 test --exclude src/tools/tidy
Build completed unsuccessfully in 0:13:08

[RalfJung]

Argh... there are a whole bunch of basic blocks being created before the codegen context is fully set up, and we really don't care about any of them as we'll exit with a hard error, but some assertion somewhere insists we give them all a terminator...

[oli-obk]

@bors r+

[bors]

📌 Commit 6d4e03a has been approved by oli-obk

[bors]

⌛ Testing commit 6d4e03a with merge 458841964cc08a5f5571285a44545d156a8a6e81...

[rust-log-analyzer]

The job dist-x86_64-linux failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

    Checking crates-io v0.31.1 (/tmp/cargo/crates/crates-io)
error[E0283]: type annotations needed
   --> src/cargo/util/config/de.rs:530:63
    |
530 |                 seed.deserialize(Tuple2Deserializer(1i32, env.as_ref()))
    |                                                           |   |
    |                                                           |   |
    |                                                           |   cannot infer type for type parameter `T` declared on the trait `AsRef`
    |                                                           this method call resolves to `&T`
    |
    = note: cannot satisfy `std::string::String: AsRef<_>`
help: use the fully qualified path for the potential candidates
    |
530 |                 seed.deserialize(Tuple2Deserializer(1i32, <std::string::String as AsRef<OsStr>>::as_ref(env)))
    |                                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
530 |                 seed.deserialize(Tuple2Deserializer(1i32, <std::string::String as AsRef<std::path::Path>>::as_ref(env)))
    |                                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
530 |                 seed.deserialize(Tuple2Deserializer(1i32, <std::string::String as AsRef<str>>::as_ref(env)))
    |                                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
530 |                 seed.deserialize(Tuple2Deserializer(1i32, <std::string::String as AsRef<[u8]>>::as_ref(env)))

error: aborting due to previous error

For more information about this error, try `rustc --explain E0283`.

[bors]

💔 Test failed - checks-actions

[RalfJung]

Same as #81375 (comment)...
@bors retry

[oli-obk]

not sure if this is retryable... I think CI is borked

[tesuji]

@oli-obk Looks like some changes in serde make type inference fails.
I can reproduce this issue locally with serde v1.0.122 in cargo-0.50.0/src/cargo/util/config/de.rs:530:63.

[RalfJung]

Yeah, the retry will take place when the tree is reopened. Cc #81378

[jonas-schievink]

Could this be the cause of the MSVC timeouts I'm seeing in #81394, #81437, and #81450?

Specifically, this test seems to hang: https://github.com/rust-lang/rust/blob/master/src/test/ui/consts/assoc_const_generic_impl.rs

[RalfJung]

That test did cause some trouble here, yeah. No idea how it can cause a hang though.
@bors rollup=never

[RalfJung]

FWIW, delete_basic_block is unsafe but I was unable to find documentation for what the safety condition is. Cc @eddyb @nikomatsakis @bjorn3 ; who else could know about this?

[bjorn3]

For cg_llvm it calls LLVMDeleteBasicBlock which calls BasicBlock::eraseFromParent which uses vector::erase to remove the basic block, thus invalidating all pointers to it. Other blocks may still reference it, so I don't think this is safe to do.

[bjorn3]

What about checking if all constant could be evaluated first before creating any basic blocks?

Edit: I didn't read #81327 (comment). Would it work if you just keep the basic blocks and never fill them?

[RalfJung]

Would it work if you just keep the basic blocks and never fill them?

I tried that first. Then I get assertion failures about missing terminators.
Maybe I should somehow add an unreachable terminator to each basic block... this is so silly.^^

[RalfJung]

Maybe I should somehow add an unreachable terminator to each basic block

Actually, that does not seem possible. There seems to be no API to switch the build to a different basic block.

[bjorn3]

I tried that first. Then I get assertion failures about missing terminators.

Then it probably tries to create an object file even when there were compilation errors before.

[RalfJung]

Possible. FWIW, the assertion failures only showed on CI, not locally (so they probably require llvm-debug-assert or so).

[RalfJung]

@bors r-
I am trying to reorder some things.

[RalfJung]

sigh yet another LLVM assertion...

Global is external, but doesn't have external or weak linkage!
void ()* @_ZN30index_out_of_bounds_never_type1f17h9ec4c246164b6bb6E
LLVM ERROR: Broken module found, compilation aborted!

[RalfJung]

@bjorn3 nope this doesn't work, it just hangs forever.

Potentially that's related to this SharedEmitterMain business? There's a channel there with messages like SharedEmitterMessage::Diagnostic and SharedEmitterMessage::AbortIfErrors; not sure who's communicating with whom there though.

[RalfJung]

At least locally, it doesn't hang if I do abort_if_errors right after compile_codegen_unit. This could mean not all post-monomorphization errors are shown if they arise in different codegen units; I am not sure how to avoid that -- every other way of using this API caused a hang.

[bjorn3]

I think this is because of the wait_for_signal_to_codegen_item() call blocking until there is a new free thread. I think you could add a !tcx.sess.has_errors() check before calling it.

[RalfJung]

Isn't it problematic to skip that blocking, and still go on building things?

[bjorn3]

The blocking is to prevent too much worker threads optimizing the codegen units at once, but because no codegen units are getting optimized anymore when an error has occured, no new worker threads are being spawned either. This means that not blocking when there was an error is fine.

[RalfJung]

This seems rather fragile; doing abort_if_errors eagerly might be easier... at least, I don't feel confident making changes like that.

For example, we are sill doing submit_pre_lto_module_to_llvm and submit_post_lto_module_to_llvm after there are errors; do those also need the !tcx.sess.has_errors() guard?

Also, if there is some invariant regarding the number of wait_for_signal_to_codegen_item calls and the number of submit_*_to_llvm calls, note that when compile_codegen_unit returns and we do the has_errors check, we have already "consumed" the "signal to codegen", so there's an off-by-1 -- no idea if that could cause problems.

[oli-obk]

@bors r+

[bors]

📌 Commit 944237f has been approved by oli-obk

[bors]

⌛ Testing commit 944237f with merge 9b32429...

[bors]

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 9b32429 to master...