regression: infallible residual could not convert error

[Mark-Simulacrum]

• https://crater-reports.s3.amazonaws.com/beta-1.54-1/beta-2021-06-23/reg/sierra-proc-demo-0.1.0/log.txt

I assume this is something subtle in the new Try trait -- would be good to investigate. Looks like only one case found in crater, so probably not too big a deal, but ideally we'd have zero regressions from the new design.

cc @scottmcm

[Mark-Simulacrum]

I'll put https://crater-reports.s3.amazonaws.com/beta-1.54-rustdoc-1/beta-2021-06-23/reg/twelf-0.1.7/log.txt under this issue for now as well since it looks like it caused by the try migration as well, though maybe distinct. Is that intentionally broken? I recall discussions about the NoneError conversions...

[the8472]

Is that intentionally broken? I recall discussions about the NoneError conversions...

Looks like it #46871 (comment)

[scottmcm]

For the sierra thing, it looks like this is a place where they unwittingly used never type on stable, and that's the core of why it changed. There was actually one place that hit this in the compiler (see the conversation in https://rust-lang.zulipchat.com/#narrow/stream/259160-t-lang.2Fproject-never-type/topic/Coercions.20of.20other.20uninhabited.20types/near/220030226) but that was a place using #![feature(never_type)] so when the previous crater run didn't hit it anywhere else I'd thought it wasn't a thing that would happen in stable :/

The code that broke is

let total_size_usize = group_size_usize
    .checked_mul(info.groups.len())
    .ok_or_else(host_memory_space_overlow)?;

where the function is

pub fn host_memory_space_overlow() -> ! {
    panic!("Memory address space overlow")
}

Repro: https://play.rust-lang.org/?version=beta&mode=debug&edition=2018&gist=f90e3876b81fc12b185284b5006457ba

It actually works fine if it's changed to

.ok_or_else(|| host_memory_space_overlow())?;

https://play.rust-lang.org/?version=beta&mode=debug&edition=2018&gist=f4b06b729fc59aa3226e64b1deaaaf4b

I'll send them a PR. (Edit: zakarumych/sierra#3 -- which is now merged.)

Aside: This is one of those spots that I think could be nice with let-else, since it would emphasize that the function called diverges:

let Some(total_size_usize) = group_size_usize
    .checked_mul(info.groups.len())
else { host_memory_space_overlow() };

---

The dhall one is known, and I sent an (incorrect) PR Nadrieril/dhall-rust#213 for it, and it was fixed back in March with Nadrieril/dhall-rust@846c14f

So twelf should also be fixed as of bnjjj/twelf@3ad9fc5 (thought it's not clear to me what version that would be)

[yaahc]

We discussed this in today's @rust-lang/libs meeting and decided that since we already approved this breakage during the RFC and the breakage has already been fixed in the repo that was caught by crater we're happy with the current status and should close this issue.

@rfcbot close

[rfcbot]

Team member @yaahc has proposed to close this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @cuviper
• @joshtriplett
• @m-ou-se
• @yaahc

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.