Skip to content

Add std.array.isSliceOf #2416

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

Add std.array.isSliceOf #2416

wants to merge 3 commits into from

Conversation

dcarp
Copy link
Contributor

@dcarp dcarp commented Aug 11, 2014

isSliceOf is a complement for sameHead and sameTail.

Returns whether $(D part) is a slice of $(D whole).
+/
@trusted
pure nothrow bool isSliceOf(T)(in T[] part, in T[] whole)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be using in so pervasively when the semantics for scope are still up in the air? Why not just use const?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in is fine here even if it does imply scope. I'm guessing the code breakage will mean that when/if scope is implemented then in will stop implying scope.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine, because here, the passed data never actually leaves the scope, so it's not just miss-use.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yebblies

I'm guessing the code breakage will mean that when/if scope is implemented then in will stop implying scope.

The problem with in and code breakage is when scope actually starts meaning something and in still means const scope, because right now, there's now difference between const and in in the vast majority of cases, but once scope is actually properly defined and implemented, then presumably in at least some of those cases, in's behavior would change due to scope changing, and if scope wasn't valid in that case or was violated, then the code would break. It could be that it's decided at some point that in will just mean const in order to avoid that kind of breakage, but it's the fact that in means const scope and scope currently does nothing but will probably do something eventually which makes it so that I almost always tell people to not use in. I honestly think that we'd be better off deprecating in than continuing to use it when its eventual semantics are unknown. So, at this point, I would never use in. That being said, in this particular case, I'd expect that in will be valid even once scope has been properly defined and implemented, since the arrays do not and cannot escape the function. So, it probably isn't a problem to use it. I still wouldn't though.

@ghost
Copy link

ghost commented Aug 11, 2014

Does it have a use-case?

@monarchdodra
Copy link
Collaborator

Does it have a use-case?

What he said. Also, does this take into account we already have overlap?
https://github.com/D-Programming-Language/phobos/blob/e7bcad9406677777183a22b19f0995a7971ba729/std/array.d#L817

I don't think

Though to be honest, the only thing I've ever had a need for is overlaps (eg: the less costly boolean version).

@dcarp
Copy link
Contributor Author

dcarp commented Aug 11, 2014

I needed a couple of times for writing contracts.
For example I want to be sure that a function does not allocate, but returns a slice of a larger array.

The overlap alternative will be too expesive for a contract:
assert(overlap(whole, part) is part);

@dcarp dcarp changed the title added std.array.isSliceOf Add std.array.isSliceOf Aug 11, 2014
@quickfur
Copy link
Member

AFAICT, overlap does essentially the same calculations as isSliceOf, since it's directly working with .ptr and .length, and isn't actually doing any array operations per se. So it doesn't seem to be anything that the optimizer can't reduce to optimal code.

@dcarp
Copy link
Contributor Author

dcarp commented Aug 14, 2014

@quickfur: overlap is not officially supported and at the moment I don't think that the optimizer can reduce as good as to the isSliceOf code. isSliceOf is complementary to the documented sameHead, sameTail. Your comments would also apply for those.

@monarchdodra
Copy link
Collaborator

Well, technically, isSliceOf provides a different functionality than overlap. I don't know what the usecase for isSliceOf is, but then again, I don't know the usecase for sameHead and sameTail either...

overlap is not officially supported and at the moment

One issue I'm seeing here is that isSliceOf (AFAIK) is subject to the same issues as overlap (not CTFE compatible), so that's a non-argument, IMO.

@DmitryOlshansky
Copy link
Member

Any conclusions - yay or nay?

@ghost
Copy link

ghost commented Sep 10, 2014

Replace in with const(T)[] (?) and we can go ahead with it I think.

@dcarp
Copy link
Contributor Author

dcarp commented Sep 10, 2014

I see no reason why isSliceOf should have other signature than sameHead/sameTail. It is worth considering an uniform API.

@monarchdodra
Copy link
Collaborator

I can't help but wonder what the use case for this function is. Not a single one was given. Without a use case, I see this as nothing more than a self serving function, so I have to say 'Nay' (sorry).

Related: An bool overlaps() is something I'd actually see some use case for. isSliceOf I don't.

@dcarp
Copy link
Contributor Author

dcarp commented Sep 10, 2014

As I said before, I need it a couple of times while writing contracts. I want to be sure that the returned array is not new allocated, but a slice of a preallocated buffer.

The overlaps usage for this usecase looks awkward:
assert(overlap(whole, part) is part);

@quickfur
Copy link
Member

@dcarp It would be nice if you turned your use case (suitably simplified, of course) into a nice, motivating ddoc'd unittest. That will not only convince reviewers, but will also serve as an excellent example of how to use this function, and more importantly, what to use it for.

@dcarp
Copy link
Contributor Author

dcarp commented Sep 11, 2014

When so many people find this PR not useful, probably it isn't... so I retract it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants