dlangBugzillaToGithub / migration_test

0 stars 0 forks source link

T[].canFind(Nullable!T()) returns true when it used to return false #1369

Open dlangBugzillaToGithub opened 1 year ago

dlangBugzillaToGithub commented 1 year ago

default_357-line (FeepingCreature) reported this on 2023-11-08T16:06:54Z

Transfered from https://issues.dlang.org/show_bug.cgi?id=24233

CC List

Description

This is a result of the behavioral change that Nullable!T now acts as a range. (Thanks Herringway!) Previously, the `opEquals` overload would allow `T == Nullable!T()` to always be false. Now, instead the overload of `canFind` that takes a range is selected, to which `Nullable!T()` acts as an empty range, so that `canFind` is vacuously true.

There is not really anything that can be done about this short of reverting the `Nullable!T` range change, which would at this point itself be a regression. Nonetheless, it *is* a regression, so might as well file it.
dlangBugzillaToGithub commented 1 year ago

default_357-line commented on 2023-11-08T16:07:50Z

To be clear, thanks Herringway for identifying the issue, not for changing Nullable.
dlangBugzillaToGithub commented 8 months ago

snarwin+bugzilla commented on 2024-03-07T22:51:14Z

This is arguably a design flaw in canFind (and find). Having different overloads of the same function take inputs with different "shapes" (range vs. element) seems like an open invitation for this kind of confusion. Perhaps something to think about for Phobos v3.
dlangBugzillaToGithub commented 8 months ago

issues.dlang commented on 2024-03-08T04:59:19Z

(In reply to Paul Backus from comment #2)
> This is arguably a design flaw in canFind (and find). Having different
> overloads of the same function take inputs with different "shapes" (range
> vs. element) seems like an open invitation for this kind of confusion.
> Perhaps something to think about for Phobos v3.

Agreed. Regardless of the issues with Nullable, there are a variety of places in Phobos where things were made too liberal in what they accepted (be it originally or by expanding their functionality later to try to make them more useful), and it's caused subtle issues and confusion like this. A similar issue is with put on output ranges accepting both singular values and ranges. It works in most cases, but in others, it results in the wrong overload being used.

Of course that doesn't necessarily mean that we shouldn't have any range-based functions accepting both elements and ranges, but it is definitely something that we need to carefully consider with each function where we're looking to do it so that we can hopefully provide the desired functionality without the subtle issues. We obviously won't catch everything, but we should be able to do better now that we know more from having used this stuff over time and folks reporting stuff like this.