fix binaryFun #2700

9il · 2014-11-13T13:45:03Z

Make binaryFun correct for integer comparison.
Remove import of half of phobos for 100% cases in phobos.

sort, cmp, find, startsWith, sum and other algorithms would be compiled faster.

See also: #2701, Issue 13253 - use more scoped imports in phobos

quickfur · 2014-11-13T16:04:44Z

std/functional.d

+        enum cmpFunR(string op) = 
+            fun == parm2Name~    op    ~parm1Name || 
+            fun == parm2Name~' '~op~' '~parm1Name;
+        static if(anySatisfy!(cmpFunR, "<", ">", "==", "!=", "+"))


Nitpick: please add a space between if and the opening parenthesis: if (...) instead of if(...). Thanks!

9il · 2014-11-13T16:49:00Z

rebased

9il · 2014-11-13T18:44:41Z

ditto

quickfur · 2014-11-13T19:29:06Z

std/functional.d

-        import std.algorithm, std.conv, std.exception, std.math, std.range, std.string;
-        auto binaryFun(ElementType1, ElementType2)
-            (auto ref ElementType1 __a, auto ref ElementType2 __b)
+        private static bool cmpFun(string op)


Should this function be put outside the binaryFun template (as a private module function)? Otherwise, it will get instantiated for every string lambda, which is a lot of template bloat.

This function use template params parm1Name and parm2Name.

Ok, i have idea.

The trick to CTFE functions, is that they can take "runtime" arguments from compile-time arguments. So you can just pass those compile-time arguments as "runtime" arguments to the function, they still get evaluated at compile-time by CTFE. :)

9il · 2014-11-13T20:21:25Z

For CTFE see private struct BinaryCmpFun

dnadlinger · 2014-11-13T21:17:49Z

std/functional.d

-            mixin("alias "~parm1Name~" = __a ;");
-            mixin("alias "~parm2Name~" = __b ;");
-            return mixin(fun);
+            // "+" needs for optimization of compilation time of summation algorithms. 


Huh? I don't understand what you are trying to say here. std.algorithm.sum doesn't seem to use binaryFun either.

Ah, okay, now I see what you are referring to, the reduce call in the default implementation of sum with seed. However, in that case, it would be enough to make sum use a lambda, wouldn't it?

reduce!"a + b" is a common code. Also lambdas is not good stuff in library because compiler can not compare them.
With lambda

auto s1 = reduce!"a + b"(ar); auto s1 = reduce!((a, b) => a + b)(ar); auto s2 = sum(ar);

will produce three reduce functions. But with sum = reduce!"a + b" only two.

Then make it a free (template) function. But in any case, that's beside the point here. I just wanted to point out that the comment is rather unclear.

Free sum?

@klickverbot please explain what I should do. My english is bad :-(

9il · 2014-11-13T22:27:41Z

Aliases removed. Rebased.

DmitryOlshansky · 2014-11-13T22:28:22Z

std/functional.d

@@ -181,6 +193,35 @@ unittest
    assert(!greater("1", "2") && greater("2", "1"));
 }

+//CTFE
+private struct BinaryCmpFun


This seems to be a crude pattern matcher rather then BinaryCmpFun which to me sounds like binary comparator function, a kind of wrapper function.

Your variant please? ;-)

MatchOperator, at least Match* part does make sense and operators are what we match against.

Other version are like MatchSimpleExpr, since this is what we handle for now.

1. Make binaryFun correct for integer comparison. 2. Remove import of half of phobos for 100% cases in phobos. `sort`, `cmp`, `find`, `startsWith`, `sum` and other algorithms would be compiled faster. fix safeOp fix safeOPs Fix new unittest fix binaryReverseArgs fix not fix new code fix spaces after if optimization 1. CTFE 2. Runtime (without inlining) change condition fix new code fix safeOp optimize CTFE CTFE optimization immutable -> enum remove aliases remove macros CTFE matching

9il · 2014-11-14T00:13:19Z

debug info:

unaryFun => (a & 1) == 0
unaryFun => sqrt(a): import std.*
unaryFun => a.sqrt: import std.*
unaryFun => .a+a: import std.*
unaryFun => _a+a: import std.*
unaryFun => a + 1
binaryFun => a < b
binaryFun => a > b
binaryFun => b+sqrt(a): import std.*
binaryFun => b+a.sqrt: import std.*
binaryFun => a[b*2]
binaryFun => a[b(a)**2]
binaryFun => a_b: import std.*
binaryFun => a.b: import std.*
binaryFun => -a < -b //added lately
binaryFun => a + b
unaryFun => a != 5
binaryFun => a != b
binaryFun => a == b
unaryFun => a
unaryFun => a * 2
unaryFun => a * 3
unaryFun => a * a
unaryFun => -a
unaryFun => to!(int)(a) + 1: import std.*

Now matching works!

DmitryOlshansky · 2014-11-14T00:29:34Z

std/functional.d

+private bool _ctfeMatchBinary(string fun, string name1, string name2)
+{
+    fun._ctfeSkipOp;
+    while ((fun._ctfeSkipName(name1) + fun._ctfeSkipName(name2) + fun._ctfeSkipInteger == 1)


Actually you can skip unary here ;)

And if you add separate stage for indexing as "primary expression" that is used in unary you'd get even further ahead in what it deduces as simple expressions such as -a < -b.

Ultimately this gets us simplistic recursive descent parser, it's only a question of balance and taste to know where to stop.

-a < -b is deduced. See debug info.

a[b(a)**2] for example ). You can check -a < -b with debug(std_functional)

I have added -a < -b in debug unittest. 9il@de82d78

9il · 2014-11-14T01:07:33Z

unittest added

quickfur · 2014-11-14T04:51:38Z

Why was this closed? Is there some blocking issue? I'd like to see this merged eventually, as it is a significant improvement IMO.

9il · 2014-11-14T05:01:52Z

I need to sleep and to think =) It will be reopened after few fixes.

DmitryOlshansky · 2014-11-14T11:46:41Z

Also check how e.g. find special cases "a == b" or was it "a==b"...

9il · 2014-11-14T22:46:08Z

@quickfur You can review.
@DmitryOlshansky tests was added.

9il · 2014-11-14T22:48:11Z

Oh, I can not reopen this PR =(

9il · 2014-11-14T22:50:02Z

Please find new PR #2723

quickfur · 2014-11-14T22:51:10Z

Hmm. Apparently you did a git push -f, and that broke GitHub's PR reopening function. Looks like you'll have to submit a new PR.

quickfur · 2014-11-14T22:51:28Z

Heh, ninja'd.

9il mentioned this pull request Nov 13, 2014

std.range.constraints & std.array code/import swap #2701

Merged

quickfur reviewed Nov 13, 2014
View reviewed changes

dnadlinger reviewed Nov 13, 2014
View reviewed changes

DmitryOlshansky reviewed Nov 13, 2014
View reviewed changes

9il added 2 commits November 14, 2014 02:40

add debug info && fix not

1274bd8

add spaces

a18f614

DmitryOlshansky reviewed Nov 14, 2014
View reviewed changes

9il added 3 commits November 14, 2014 03:29

minor fix

bf7c523

added debug example

de82d78

add debug unittests & fix

21b69bc

9il closed this Nov 14, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix binaryFun #2700

fix binaryFun #2700

9il commented Nov 13, 2014

quickfur Nov 13, 2014

9il commented Nov 13, 2014

9il commented Nov 13, 2014

quickfur Nov 13, 2014

9il Nov 13, 2014

9il Nov 13, 2014

quickfur Nov 13, 2014

9il commented Nov 13, 2014

dnadlinger Nov 13, 2014

dnadlinger Nov 13, 2014

9il Nov 13, 2014

dnadlinger Nov 13, 2014

9il Nov 13, 2014

9il Nov 13, 2014

9il commented Nov 13, 2014

DmitryOlshansky Nov 13, 2014

9il Nov 13, 2014

DmitryOlshansky Nov 13, 2014

DmitryOlshansky Nov 13, 2014

9il commented Nov 14, 2014

DmitryOlshansky Nov 14, 2014

9il Nov 14, 2014

9il Nov 14, 2014

9il Nov 14, 2014

9il commented Nov 14, 2014

quickfur commented Nov 14, 2014

9il commented Nov 14, 2014

DmitryOlshansky commented Nov 14, 2014

9il commented Nov 14, 2014

9il commented Nov 14, 2014

9il commented Nov 14, 2014

quickfur commented Nov 14, 2014

quickfur commented Nov 14, 2014

fix binaryFun #2700

fix binaryFun #2700

Conversation

9il commented Nov 13, 2014

Choose a reason for hiding this comment

9il commented Nov 13, 2014

9il commented Nov 13, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

9il commented Nov 13, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

9il commented Nov 13, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

9il commented Nov 14, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

9il commented Nov 14, 2014

quickfur commented Nov 14, 2014

9il commented Nov 14, 2014

DmitryOlshansky commented Nov 14, 2014

9il commented Nov 14, 2014

9il commented Nov 14, 2014

9il commented Nov 14, 2014

quickfur commented Nov 14, 2014

quickfur commented Nov 14, 2014