Skip to content

Conversation

monarchdodra
Copy link
Collaborator

Partial fix for 11403:
http://d.puremagic.com/issues/show_bug.cgi?id=11403

This pull allows declaring any, all and canFind with their predicates, but without args. This allows either aliasing them to a new name, or using them as a pred to yet a third function. For example:

alias fun = canFind!"a < b";
if (fun([1, 2, 3], 4))
{...}

or

import std.ascii : isWhite;
assert( all!(any!isWhite)(["a a", "b b"]));
assert(!any!(all!isWhite)(["a a", "b b"]));

@ghost
Copy link

ghost commented Nov 1, 2013

I wish this was a language feature. I think I've filed this as an enhancement somewhere.

@monarchdodra
Copy link
Collaborator Author

I think I've filed this as an enhancement somewhere.

https://d.puremagic.com/issues/show_bug.cgi?id=9365

I wish this was a language feature.

Hum...

Andrej Mitrovic 2013-01-21 13:15:33 PST
This can be implemented in the library, the last thing you want is for
templates being partially instantiated when you pass too few arguments instead
of getting an error.

I think partial instantiation is specifically not supported no? It comes with its own problems, like ambiguity ( is foo!int the full foo(T) or a partial foo(T, U)?). There's also the issue with constraints, which require having all the parameters known for evaluation: If we decide to wait until all the parameters are known, then we could end up with ambiguity on declaration.

I should be pasting this in the issue instead of here.

In any case, I think my fix is rational and stand alone.

@ghost
Copy link

ghost commented Nov 10, 2013

Right right, maybe I'd like some special syntax for it, anyway I was thinking out loud. The pull is a welcome change.

@monarchdodra
Copy link
Collaborator Author

Turns out that an overzealous constraint prevented canFind from working with Elements..., where elements is not a range, eg:

assert(canFind([0, 1, 2, 3], 1, 3) == 1);

So I fixed that too while I was at it.

@monarchdodra
Copy link
Collaborator Author

I just learned that it turns out that eponymous templates can be without specifying parameters (unlike normal templates, where you need to !()). This means I was able to keep the code much cleaner, while handling both with/without pred in a single statement:

template canFind(alias pred = "a == b")
{
    auto canFind(R, E)(R r, E e);
}

//Both of these work
canFind([1, 2, 3], 2);//Pred implicitly evaluated as "a == b"
canFind!"a > b"([1, 2, 3], 2);//Pred explicitly given

So that's cool.

@AndrejMitrovic : Thoughts/ping?

@ghost
Copy link

ghost commented Nov 27, 2013

LGTM. I would have wanted to check out the ddoc output but alas...

@MartinNowak
Copy link
Member

I just learned that it turns out that eponymous templates can be without specifying parameters

Works like normal function templates, right?

LGTM, will check the docs tomorrow.

{
return !find!pred(range).empty;
/// ditto
Copy link
Member

Choose a reason for hiding this comment

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

That doesn't work correctly for the docs. Seems like you need to document both entities, the template and the template function.
algo

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah. So I just mark it as /// ?

Copy link
Member

Choose a reason for hiding this comment

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

That doesn't work with ddox, might be a bug though. Maybe you can split the documentation.
Maybe something like "Returns true is a value ... can be found." and "Implementation for input ranges."?

@MartinNowak
Copy link
Member

I wish this was a language feature. I think I've filed this as an enhancement somewhere.

Where, it would be fairly easy to simply allow more parens.
The if-condition would only apply to the innermost template though.

bool any(alias pred)(Range)(Range range) if (is(typeof(find!pred(range))))
{
    return return !find!pred(range).empty;
}
template any(alias pred)
{
    template any(Range) if (is(typeof(find!pred(range))))
    {
        bool any(Range range)
        {
            return return !find!pred(range).empty;
        }
    }
}

@monarchdodra
Copy link
Collaborator Author

Works like normal function templates, right?

I meant as opposed to non-eponymous:

template Foo()
{
  void bar();
}
void main()
{
  Foo.bar();
}

{
return !find!pred(haystack, needle).empty;
}
//Explictly Undocumented. Do not use. It may be deprecated in the future.
Copy link
Member

Choose a reason for hiding this comment

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

If this is undocumented, why did you use /++ above?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@andralex : The first /++ documents the canFind(alias pred="a == b") familly of functions, which contains 3 overloads:

canFind(alias pred="a == b").canFind(Range)(Range haystack); //NOT documented
canFind(alias pred="a == b").canFind(Range, Element)(Range haystack, Element needle); //documented
size_t canFind(Range, Elements...)(Range haystack, Elements needles); //documented

I need to double check how it works though, according to @MartinNowak , the generated DDoc isn't correct.

Copy link
Member

Choose a reason for hiding this comment

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

Well, this works as expected, the template is documented as well as two of the three overloads.

@monarchdodra
Copy link
Collaborator Author

OK: I've added the assert (also noticed the unittest was actually wrong...), and reworked the documentation of all/any. Should be better now.

MartinNowak added a commit that referenced this pull request Dec 19, 2013
Issue 11403 - functions in std.algo can't be used as pred
@MartinNowak MartinNowak merged commit a5279fa into dlang:master Dec 19, 2013
@monarchdodra
Copy link
Collaborator Author

Cool, thanks. Now, I gotta apply the same reasoning to a couple other functions.

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.

3 participants