Stream: t-compiler/help

Topic: typeck: demand.rs


Ram (Apr 09 2020 at 12:40, on Zulip):

This is for #70851. How exactly would I add a condition for 16/8 bits?
Sorry that it's a newbie question, but this is my first contribution and I'm not very familiar with the project yet.

LeSeulArtichaut (Apr 09 2020 at 13:23, on Zulip):

I guess a match guard (this thing) could fit well

Ram (Apr 09 2020 at 13:43, on Zulip):

Yes thank you, I'm using that. I should have been clearer. My question is about how do I know if found or exp is a 8 bit int? Is the condition to see if it's Some(8) for the 8 bits, or Some(1) for the 1 byte, or simply 8 or 1? And what would I be comparing it with?
If I compare it with found.bit_width(), I will need to use Some(8) and see if they're equal for example.

LeSeulArtichaut (Apr 09 2020 at 13:45, on Zulip):

See the definition of bit_width

LeSeulArtichaut (Apr 09 2020 at 13:46, on Zulip):
pub fn bit_width(&self) -> Option<u64> {
        Some(match *self {
            IntTy::Isize => return None,
            IntTy::I8 => 8,
            IntTy::I16 => 16,
            IntTy::I32 => 32,
            IntTy::I64 => 64,
            IntTy::I128 => 128,
        })
    }
LeSeulArtichaut (Apr 09 2020 at 13:46, on Zulip):

So it is the number of bits, you'll need to use Some(8) rather than Some(1)

LeSeulArtichaut (Apr 09 2020 at 13:49, on Zulip):

And what would I be comparing it with?

If I'm right, the issue here is that conversions from 32+ bit integer are failible but from 8 and 16 bits integer, they are infailible.

LeSeulArtichaut (Apr 09 2020 at 13:50, on Zulip):

So you would simply need to compare with 16 to see if the comparison is fallible or not.

Ram (Apr 09 2020 at 15:21, on Zulip):

Great, thank you!
Do you think there's actual use in adding a match case of
(Some(8), None) => true, as that's going to be caught by the (_, _) anyway? Maybe it's the other way around?

LeSeulArtichaut (Apr 09 2020 at 15:28, on Zulip):

I think the best way to do this is to add a match guard to the already existing arm

LeSeulArtichaut (Apr 09 2020 at 15:29, on Zulip):

This one:

(None, _) | (_, None) => true,
LeSeulArtichaut (Apr 09 2020 at 15:31, on Zulip):

So that we only take it if the integer is greater than 16 bits

LeSeulArtichaut (Apr 09 2020 at 15:32, on Zulip):

Ping @Ram

Ram (Apr 09 2020 at 16:23, on Zulip):

Thanks! Would something like

 (None, Some(8))  | (Some(8), None)  | (None, Some(16))| (Some(16), None)   => false

be a cleaner way of expressing the same?

centril (Apr 09 2020 at 16:24, on Zulip):

You can rewrite that as:

 (None, Some(8 | 16))  | (Some(8 | 16), None)  => false
centril (Apr 09 2020 at 16:25, on Zulip):

I wish we could have written that as Xor(None, Some(8 | 16)) => false

bjorn3 (Apr 09 2020 at 16:32, on Zulip):

Option has an xor method.

centril (Apr 09 2020 at 16:34, on Zulip):

@bjorn3 yeah, good to use here

LeSeulArtichaut (Apr 09 2020 at 16:38, on Zulip):

We could also write:

(None, Some(size)) | (Some(size), None) if size > 16 => true,
centril (Apr 09 2020 at 16:38, on Zulip):

@LeSeulArtichaut I don't think that's better; it's more imprecise, and in the general case, exhaustiveness checking does not work as you have a guard

LeSeulArtichaut (Apr 09 2020 at 16:39, on Zulip):

Right, in this case it's probably not better

LeSeulArtichaut (Apr 09 2020 at 16:40, on Zulip):

But if we had a bigger number of possible values, encoding them in patterns is painful :slight_smile:

centril (Apr 09 2020 at 16:41, on Zulip):

@LeSeulArtichaut depends on specifics, but you have range-patterns and or-patterns at your disposal to make it not painful

centril (Apr 09 2020 at 16:41, on Zulip):

In general, I'd say avoid guards if it is encodable in the pattern and it's not an obvious win

LeSeulArtichaut (Apr 09 2020 at 16:42, on Zulip):

Alright, good to know

centril (Apr 09 2020 at 16:42, on Zulip):

(ofc that's my opinion, which reasonable people may disagree with ^^)

Ram (Apr 09 2020 at 17:05, on Zulip):

centril said:

(None, Some(8 | 16)) | (Some(8 | 16), None) => false

I wish we could have written that as Symmetrical(None, Some(8 | 16)) => false

Thanks @centril ! TIL
Yeah, that would be a neat way to do it.

centril (Apr 09 2020 at 17:05, on Zulip):

@Ram you'll need to add #![feature(or_patterns)] to the crate if it ain't already there btw (please do, we want to test the feature internally!)

Ram (Apr 09 2020 at 17:32, on Zulip):

Is this what you meant? I guess it already exists.
https://github.com/rust-lang/rust/blob/master/src/librustc_typeck/lib.rs#L65

centril (Apr 09 2020 at 17:32, on Zulip):

@Ram yep; so nothing needs fixing there :slight_smile:

Ram (Apr 09 2020 at 17:32, on Zulip):

BTW, is there a reason why we don't want the compiler message for 32 and 64 bit ints to be fixed too?

Ram (Apr 09 2020 at 18:10, on Zulip):

Also do we want it this way whether the fn requires u8 but is given a u8 in the return; or the fn requires usize but is given u8 by the return.
Is it working both ways the requirement here?

Ram (Apr 10 2020 at 11:52, on Zulip):

I think this is a better way of phrasing those questions.

Is there a reason why we suggest 8 and 16 bit ints to use .into() but not 32/64 bit ones?

In the example given in the top issue comment(linked below), should it be such that the compiler suggests .into() when the fn requires a u8 but is supplied a usize instead as well? The example given is that of the fn requiring a usize but supplied a u8 instead. The difference being that u8 is likely to be < usize (but is not guaranteed anyway).

https://github.com/rust-lang/rust/issues/70851#issue-595356744

Jonas Schievink (Apr 10 2020 at 11:59, on Zulip):

@Ram There are no From impls for u32/u64 -> usize

Jonas Schievink (Apr 10 2020 at 11:59, on Zulip):

Because usize might be 16 bits in size for small targets like MSP430 and AVR

Vecr (Apr 10 2020 at 12:01, on Zulip):

I think Rust currently defines usize/isize to be from 16 bits to 128+ bits.

LeSeulArtichaut (Apr 10 2020 at 12:02, on Zulip):

You can see the From and TryFrom implementations for usize/isize by the way

Ram (Apr 10 2020 at 12:10, on Zulip):

usize depends on the target it's built for iirc so could it be 8 bits or less for some embedded systems @Vecr ?
Thanks @LeSeulArtichaut https://doc.rust-lang.org/src/core/convert/num.rs.html#48-53

Vecr (Apr 10 2020 at 12:12, on Zulip):

I think usize implements From<u16>, but it might not on some platforms.

Jonas Schievink (Apr 10 2020 at 12:12, on Zulip):

The minimum is 16 bits, so From<u16> and From<u8> are still implemented for usize

Jonas Schievink (Apr 10 2020 at 12:12, on Zulip):

Systems with 256 bytes of address space are not exactly useful

eddyb (Apr 10 2020 at 14:16, on Zulip):

@Jonas Schievink don't let the dozens of 8051's deeply embedded in your devices hear you say that

Jonas Schievink (Apr 10 2020 at 14:16, on Zulip):

Even that has a 16-bit address bus :P

eddyb (Apr 10 2020 at 14:17, on Zulip):

8-bit RAM, 16-bit ROM, I guess

eddyb (Apr 10 2020 at 14:23, on Zulip):

@Jonas Schievink oh dear https://www.dcd.pl/product/dq80251/

Jonas Schievink (Apr 10 2020 at 14:23, on Zulip):

World’s Fastest 8051 IP Core

oh no

Ram (Apr 11 2020 at 03:32, on Zulip):

With the changes I've made now, it's such that the compiler will suggest usize be .into() when the function requires a u8.
Is that fine, because usize will greater than u8 on most targets?
And From(usize) is not implemented for u8/u16 anyway so that would be a bad suggestion.

Jonas Schievink (Apr 11 2020 at 09:31, on Zulip):

Yes, usize cannot be u8

Ram (Apr 11 2020 at 10:57, on Zulip):

Yes, so would
(None, Some(8|16)) be the correct solution rather than
(None, Some(8|16)) | (Some(8|16), None)
because this doesn't suggest .into() when a usize is the expected type at all?
Sorry if I'm not being clear :sweat_smile:

LeSeulArtichaut (Apr 11 2020 at 11:03, on Zulip):

@Ram I think you're right. I left you a comment on GitHub :slight_smile:

Last update: Sep 28 2020 at 15:45UTC