Stream: t-libs

Topic: should String::drain() w/ overflow incl range panic #72237


pnkfelix (May 21 2020 at 18:50, on Zulip):

hi @T-libs . We discussed #72237 at the T-compiler meeting today, and there was some contention as to whether this was even a bug at all.

Steven Fackler (May 21 2020 at 18:51, on Zulip):

what's the argument for it not being a bug?

pnkfelix (May 21 2020 at 18:51, on Zulip):

i wish @Esteban Küber had spelled their opinion here out more

pnkfelix (May 21 2020 at 18:52, on Zulip):

or oh

pnkfelix (May 21 2020 at 18:52, on Zulip):

maybe @Esteban Küber 's point wasn't about whether the current behavior is buggy

pnkfelix (May 21 2020 at 18:52, on Zulip):

but rather about whether it would be better to DWIM here

Esteban Küber (May 21 2020 at 18:52, on Zulip):

The latter

pnkfelix (May 21 2020 at 18:52, on Zulip):

this comment:

If we change the impls of drain to accept this off by one case we can make it work as the original code intended, otherwise itd be much nicer for it to be a compile time error instead of a panic. If we want it to be a panic then the patch is minimal

pnkfelix (May 21 2020 at 18:53, on Zulip):

so okay, that's definitely a T-libs call

Esteban Küber (May 21 2020 at 18:53, on Zulip):

The entire point of ..= existing is to avoid these off by one errors, after all :sweat_smile:

Steven Fackler (May 21 2020 at 18:53, on Zulip):

it should definitely panic if it compiles, yeah

Steven Fackler (May 21 2020 at 18:54, on Zulip):

there could be a lint for "statically out-of-bounds bounds" but I don't feel super strongly about that

simulacrum (May 21 2020 at 18:58, on Zulip):

I mean, there is in some sense -- if we detect it, we'll say something like "this code will panic at runtime" -- but I imagine we'll not get there for a long time :)

pnkfelix (May 21 2020 at 19:01, on Zulip):

@Esteban Küber i fear that I am either not understanding your proposal or not communicating it properly

pnkfelix (May 21 2020 at 19:01, on Zulip):

@Esteban Küber can you state what you mean, in the "DWIM" case here?

pnkfelix (May 21 2020 at 19:02, on Zulip):

i.e., by "accept this off by one case", I take that to mean: "there is a sensible non-panicking and non-erroring semantics here. Use it."

pnkfelix (May 21 2020 at 19:03, on Zulip):

but I would prefer if you wrote out your intention rather than have me attempt to infer it or reconstruct it.

Last update: Jul 03 2020 at 15:50UTC