pyret-lang icon indicating copy to clipboard operation
pyret-lang copied to clipboard

Error in type of member in documentation

Open kfisler opened this issue 2 years ago • 9 comments

The type given for member in the docs constrains the element to search for to be of the same type that is in the list, rather than Any, which works in practice.

https://www.pyret.org/docs/latest/lists.html#%28part._lists_member%29

kfisler avatar Feb 20 '23 17:02 kfisler

Wasn't that deliberate, though? After all if the types aren't the same, the value can't possibly be in the list. The implementation can't check at runtime that the 'a types are the same (since we can't check parametric type variables anyway), but if the static type system can rule out the program as being type-invalid, then we thought that was a teachable moment of "oh duh, we don't need to run .member here at all!"

blerner avatar Feb 20 '23 20:02 blerner

That's fine. I certainly appreciate this type-checking argument. It just isn't consistent with what we can run

kfisler avatar Feb 20 '23 21:02 kfisler

@kfisler are you arguing that the docs should change or the implementation should change? And: how?

shriram avatar Feb 21 '23 00:02 shriram

I don't know what the implementation is. All I know is that I can call [list:1, 2, 3].member("a"), getting false as expected. The documentation appears inconsistent with my ability to make that call to a reader who isn't thinking about static typechecking (which was the source of the typo report that we got on DCIC around this issue, since DCIC says that the contract on member is List<A>, Any -> Boolean.

Personally, the Any type makes sense to me here, but you've all thought about the implementation implications

kfisler avatar Feb 21 '23 00:02 kfisler

But there are always going to be things that a dynamically typed language will permit that a statically typed one wont, simple as link(1, link(true, link("hi", empty))). That doesn't mean we're going to change the type of lists to be heterogeneous!

blerner avatar Feb 21 '23 00:02 blerner

Ben, might it not be better if the documentation stated the intended behavior? I am not a fan of Any in this setting.

shriram avatar Feb 21 '23 01:02 shriram

I think this falls into the category of “won’t check dynamically, too expensive, will enforce statically”. It’s a design point we’ve pretty deliberately ended up at.

I can’t quickly find a good docs link to explain what gets checked and what doesn’t statically vs dynamically, which is maybe the thing to fix and reference from parametric List methods

But I don’t think we have any way to sensibly enforce this dynamically in general.

It is a static error:

On Mon, Feb 20, 2023 at 4:57 PM Ben Lerner @.***> wrote:

But there are always going to be things that a dynamically typed language will permit that a statically typed one wont, simple as link(1, link(true, link("hi", empty))). That doesn't mean we're going to change the type of lists to be heterogeneous!

— Reply to this email directly, view it on GitHub https://github.com/brownplt/pyret-lang/issues/1685#issuecomment-1437716455, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA5IUYWVRDM3N5QE5MEERDWYQHIZANCNFSM6AAAAAAVCEQPRM . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- This message was sent from my phone.

jpolitz avatar Feb 21 '23 01:02 jpolitz

Okay, so maybe we should just edit the reference to Any in DCIC and gloss over the fact that actually any input argument would work.

On Mon, Feb 20, 2023, 8:02 PM Joe Politz @.***> wrote:

I think this falls into the category of “won’t check dynamically, too expensive, will enforce statically”. It’s a design point we’ve pretty deliberately ended up at.

I can’t quickly find a good docs link to explain what gets checked and what doesn’t statically vs dynamically, which is maybe the thing to fix and reference from parametric List methods

But I don’t think we have any way to sensibly enforce this dynamically in general.

It is a static error:

On Mon, Feb 20, 2023 at 4:57 PM Ben Lerner @.***> wrote:

But there are always going to be things that a dynamically typed language will permit that a statically typed one wont, simple as link(1, link(true, link("hi", empty))). That doesn't mean we're going to change the type of lists to be heterogeneous!

— Reply to this email directly, view it on GitHub < https://github.com/brownplt/pyret-lang/issues/1685#issuecomment-1437716455 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAA5IUYWVRDM3N5QE5MEERDWYQHIZANCNFSM6AAAAAAVCEQPRM

. You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- This message was sent from my phone.

— Reply to this email directly, view it on GitHub https://github.com/brownplt/pyret-lang/issues/1685#issuecomment-1437719701, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALO7SKA66DNE3Q3IWCIMIDWYQH33ANCNFSM6AAAAAAVCEQPRM . You are receiving this because you were mentioned.Message ID: @.***>

kfisler avatar Feb 21 '23 01:02 kfisler

I'm not sure whether you [@jpolitz] are disagreeing with me or think I'm disagreeing with you or what.

My position:

  • We cannot afford to check this dynamically. [NB: We still check the list tag, not its contents.]
  • We can afford to check it in statically-typed Pyret.
  • But the published contract is still misleading. I do not like the Any there.

So my proposal is that the documentation say it's a homogenously-typed list, but leave the enforcement to statics, maybe an expensive dynamic mode (currently non-existent), or vibes (current dynamic mode).

DCIC should pretend like this is type-enforced. After all, it is enforced in current Anchor: [list: 1, 2, 3].member("a") gives a static type error.

shriram avatar Feb 21 '23 01:02 shriram