phobos icon indicating copy to clipboard operation
phobos copied to clipboard

Add isSliceOf to check the origin of a slice

Open wilzbach opened this issue 6 years ago • 18 comments

This is has come up before:

  • https://github.com/dlang/phobos/pull/2416
  • https://forum.dlang.org/post/[email protected]

tl;dr: of the previous discussion:

  • overlap exists, but is confusing to use
  • there were no "real world" use cases / huge interest
    • @dcarp provided some and I have run into similar uses when working with allocators (it's analog to owns)
    • sameHead and sameTail already exists
    • @trusted doesn't work for one-liners

CC @nordlow @dcarp

wilzbach avatar Feb 09 '18 14:02 wilzbach

Thanks for your pull request, @wilzbach!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#6147"

dlang-bot avatar Feb 09 '18 14:02 dlang-bot

std.exception.doesPointTo?

JackStouffer avatar Feb 09 '18 14:02 JackStouffer

std.exception.doesPointTo?

Sorry for not mentioning that one. It's pretty useless (and in the wrong module):

If target is a pointer, a dynamic array or a class, then these functions will only check if source points to target, not what target references.

Convince yourself: https://run.dlang.io/is/NoZVWq

wilzbach avatar Feb 09 '18 14:02 wilzbach

Argh, the spuriously failing std.regex due to its high memory consumption is getting more and more annoying.

See also: https://issues.dlang.org/show_bug.cgi?id=18411

wilzbach avatar Feb 09 '18 19:02 wilzbach

Any reason to not use the &whole[0] style rather than whole.ptr?

JackStouffer avatar Mar 16 '18 17:03 JackStouffer

Any reason to not use the &whole[0] style rather than whole.ptr?

Yes - isSliceOf should work for empty arrays too, e.g.

assert(null.isSliceOf(null));

wilzbach avatar Apr 06 '18 14:04 wilzbach

null isn't a slice of itself because null isn't anything. Nothing can't be a part of nothing.

JackStouffer avatar Apr 06 '18 15:04 JackStouffer

Nothing can't be a part of nothing.

So you would treat null like FP's NaN where the default value is poison and yields false? Interesting thought! The problem here is that null is null, so I thought it makes sense that isSliceOf matches this behavior.

wilzbach avatar Apr 06 '18 15:04 wilzbach

If I had to be pedantic, I would say the nothing is the same as nothing but nothing is not a memory view of nothing.

JackStouffer avatar Apr 06 '18 18:04 JackStouffer

Tangential to this pull request:

overlap exists, but is confusing to use

Maybe the documentation needs improvement then, because if (a.overlap(b)) should be crystal clear!

n8sh avatar Apr 10 '18 20:04 n8sh

Ping @andralex

thewilsonator avatar Dec 02 '18 03:12 thewilsonator

This API could be better. As it is, it returns very little information i.e. "is this range overlapping this other range in any position?" with no further information on position.

The same information can be trivially fetched using overlap in one of two ways:

small.overlap(large) is small

or

small.overlap(large).length == small.length

I like the first one more for generality, and the second more for efficiency. Given the rarity of such a query, I think we don't need a new function for it.

BUT! All is not lost. I propose to convert this PR into a documentation PR. The definition and use of isSliceOf can be put in a documentation unittest. That way we match google queries like "dlang isSliceOf" and similar. Win-win-win!

andralex avatar Dec 19 '18 18:12 andralex

IMO your suggestion fall foul of

overlap exists, but is confusing to use

If I saw small.overlap(large) is small it would take me a good while to figure out what that meant.

As it is, it returns very little information i.e. "is this range overlapping this other range in any position?" with no further information on position.

c.f. canFind(/find), so what? The index is trivially whole.ptr - part.ptr but you'd just use part.ptr.

Given the rarity of such a query, I think we don't need a new function for it.

@dcarp provided some and I have run into similar uses when working with allocators (it's analog to owns)

The existence of one-liners on phobos exist precisely to a) stop people reinventing the wheel, and b) use a common name: this fit both of those.

Please reconsider.

thewilsonator avatar Dec 20 '18 22:12 thewilsonator

The existence of one-liners on phobos exist precisely to a) stop people reinventing the wheel, and b) use a common name: this fit both of those.

(c) must be in frequent use, this doesn't pass that test

andralex avatar Dec 20 '18 22:12 andralex

If I saw small.overlap(large) is small it would take me a good while to figure out what that meant.

"The overlap of small and large is the same as small itself, i.e. large subsumes small." For somebody in a place in life where they need overlap, this is not quite the leap.

andralex avatar Dec 20 '18 22:12 andralex

For somebody in a place in life where they need overlap, this is not quite the leap.

I agree, but for those not in need of or familiar with overlap, small.overlap(large) is small is obtuse.

thewilsonator avatar Dec 20 '18 23:12 thewilsonator

@thewilsonator wouldn't disagree. I still think this should be relegated to a documentation example. Thanks.

andralex avatar Dec 20 '18 23:12 andralex

@andralex I have put isSliceOf into a documented unittest and have used overlap instead of pointer arithmetics. However, I had to delete a few tests regardind the behavior in the presence of null. However, I think this is fine since the point is to provide a use case for overlap.

RazvanN7 avatar May 17 '22 11:05 RazvanN7