Fix predicate template implementations in std.traits and std.range #842

9rnsr · 2012-10-06T04:45:13Z

Use T.init property instead of void initializer, because it doesn't work if T is const or immutable type.

I must change a part of unit test for the bug 6935, because we cannot support ranges which overrides init property.

From the mention by @donc, using = void in predicate is sometimes not good. Then we should use T.init or library solution (e.g. defaultInit in std.traits).

For almost cases, T.init is enough. But, void static array types don't have .init property. Then, if we want to treat all types, defaultInit is necessary I think.
(Furthermore, it seems to me acceptable that default initialized void static arrays have zero filled memory. But it is language spec issue.)

andralex · 2012-10-06T06:56:09Z

std/range.d

@@ -551,7 +551,7 @@ template isInputRange(R)
    enum bool isInputRange = is(typeof(
    (inout int _dummy=0)
    {
-        R r = void;       // can define a range object
+        R r = R.init;     // can define a range object


Is there a difference between this and R r;?

R r; fails if R is a struct with disabled default construction.

Are you asking whether or not such structs should be valid ranges?

If R has @disable this();, R r; is not allowed.
To solve the problem, I had changed to R r = void;, but it has still problem if R is const or immutable type.
(const/immutable range is almost meaningless, but it is syntactically allowed). Then, use R.init is better.

I see, thanks. I find it very arcane to rely on this kind of trickery to distinguish between types with disabled default constructor and the others. Any other ideas for a simpler way?

andralex · 2012-10-07T03:03:58Z

I just realized something - there are two ways to create a value of a non-copyable type:

Define a pointer and then use it.

(*cast(T*) null)

That works great if the only need is to check whether something compiles without having to construct an object

Put it in a union.

union { T t; };

Putting anything in a union should guarantee no constructor or destructor is ever called. Currently the compiler doesn't do that, there's a bug report about that: http://d.puremagic.com/issues/show_bug.cgi?id=4421.

monarchdodra · 2012-10-07T16:03:38Z

Just want to point out: http://d.puremagic.com/issues/show_bug.cgi?id=8762

I propose that template defaultInit(T) be simply re-written as:
@property ref T defaultInit( T )( );

It is guaranteed to work for ALL types T, because everything is in the signature (no complicated hasWild, or gotcha's). It cannot be erroneously used at run-time, because un-implemented, so will link error.

At this point, the implementations can safely use, no questions asked:

template isOutputRange(R, E)
{
    enum bool isOutputRange = is(typeof(
    (inout int _dummy=0)
        R r = defaultInit!R;
        E e = defaultInit!E;
        put(r, e);
    }));
}

I think it is a good proposal, no?

Heck, one can then even dispense the declaration of r/e, and do a straight up:

template isOutputRange(R, E)
{
    enum bool isOutputRange = is(typeof(put(defaultInit!R, defaultInit!E)));
}

Which also has the advantage of ditching the whole _dummy function too...

Added bonus, because it is a function, it can even work on a value type, making it easy to extract a value out of an LValue out of an expression:

auto bar(T)(){???}

//Checks if the type returned by bar is assignable from U
template foo(T, U)
{

    enum bool foo = is(typeof( bar!T().defaultInit = defaultInit!U))
}

PS: defaultInit is private right now, so I won't complain, but if it ever becomes public, I really think instanceOf or instanceof is a much better name for it.

andralex · 2012-10-14T15:25:23Z

I think defaultInit defines yet another concept that people would need to grasp. Why not use init instead with the same semantics?

9rnsr · 2012-10-14T15:53:13Z

Because a void static array type (e.g. void[1]) doesn't have init property, so we should care it in library.
defaultInit is a utility to avoid such problems and a kind of hack for current compiler behavior.

9rnsr · 2012-10-14T15:53:55Z

Furthermore, (void[1]).init should be valid IMO, even if it's a language change.

alexrp · 2012-10-19T09:32:20Z

std/traits.d

@@ -112,14 +112,50 @@ private
    alias TypeTuple!(IntegralTypeList, FloatingPointTypeList) NumericTypeList;
    alias TypeTuple!(char, wchar, dchar) CharTypeList;

-    /* Get an expression typed as T, like T.init */
+    template hasWild(T)


I know this is terminology from DMD, but I think hasInout is better.

alexrp · 2012-10-19T09:32:59Z

@9rnsr should we move forward with this or wait for a compiler change to allow (void[1]).init?

9rnsr · 2012-12-05T00:52:21Z

Now, I have proposed a language enhancement issue 8819 for void[1].init, and implemented it in dlang/dmd#1324 .

alexrp · 2013-03-08T17:51:16Z

@9rnsr this needs a rebase

9rnsr · 2013-03-09T01:38:25Z

Finally, by fixing issue 8819, all types have .init property right now. So no trick is necessary. We can use T.init in meta programming at all!

ghost · 2013-03-09T01:43:52Z

std/traits.d

@@ -113,17 +113,55 @@ private
    alias TypeTuple!(cfloat, cdouble, creal) ComplexTypeList;
    alias TypeTuple!(IntegralTypeList, FloatingPointTypeList) NumericTypeList;
    alias TypeTuple!(char, wchar, dchar) CharTypeList;
+
+    template hasInout(T)


Hmm.. we should have some kind of TargetType template which does what this template does, and then this kind of template would be implemented as static if (is(TargetType!T == inout)). ElementType comes close but I'm not sure if it works with pointers and associative arrays. This would avoid code duplication for future templates like this one.

So this is undocumented for now?

Yes. I'd like to keep internal templates private until it is really needed.

Fine with me.

jmdavis · 2013-03-09T01:52:56Z

Finally, by fixing issue 8819, all types have .init property right now. So no trick is necessary. We can use T.init in meta programming at all!

So, does that mean that just meant that T.init always works in metaprogramming, or does it mean that all types have T.init regardless? Is it now impossible to disable init?

9rnsr · 2013-03-09T02:08:00Z

I think T.init should always work in metaprogramming (of course, void is only one exception of the rule).
So, I mean that all types should have .init property regardless.
However, I agree that using .init is sometimes unsafe (Related: issue 8752). Compiler should provide init for all types and it should be usable in typeof code, but restricting other use outside typeof is possible.

denis-sh · 2013-10-02T21:53:20Z

#1261 is merged and defaultInit is called rvalueOf now. Also T.init looks working everywhere. So lets chose a replacement for = void, rebase and merge.

monarchdodra · 2013-10-03T06:45:35Z

So lets chose a replacement for = void, rebase and merge.

Just want to point out we also have lvalueOf now, so we can get away with no declaring the variables now. As in at all:

    is(typeof(
    (inout int = 0)
    { 
        R r = R.init;
        E e;
        put(r, e);
    }

Could simply be:

    is(typeof(
    (inout int = 0)
    { 
        put(lvalueOf!R, rvalueOf!E);
    }

Worst case scenario, if we need the variable more than once:

        alias r = lvalueOf!R;
        r.popBack();
        auto t = r.back;
        auto w = r.front;

denis-sh · 2013-10-03T07:42:55Z

Worst case scenario, if we need the variable more than once:
...

Didn't think about that. Looks clear and consistent especially with new alias syntax!

ghost · 2013-10-24T13:18:59Z

Status of this?

monarchdodra · 2013-10-24T14:14:03Z

Status of this?

The whole things looks obsolete to me now (IMO). lvalueOf solves the entire problem, while also dealing with inout.

9rnsr · 2013-11-04T03:45:33Z

I reopen this PR, because using lvalueOf is overkill for std.range module. Using .init property is much more lightweight.

9rnsr · 2013-11-04T03:46:04Z

@andralex Ping.

DmitryOlshansky · 2014-01-25T12:42:28Z

@9rnsr @andralex
Any advise?
I see this is now there is 2 matters:

using the now public rvalueOf instead of undocumented defaultInit
Using .init instead of void or maybe using lvalueOf inside of std.range traits.

If anything I'd say go for public primitives in the implementation (else what good they are).
And that is lvalueOf and rvalueOf.

9rnsr · 2014-01-25T16:32:08Z

using the now public rvalueOf instead of undocumented defaultInit

defaultInit is now unnecessary because it was a hack for the compiler bugs around T.init. I removed it.

Using .init instead of void or maybe using lvalueOf inside of std.range traits.

I think using rvalueOf in std.range traits is not good. (Note that using lvalueOf is not correct) Because:

It will add a dependency from std.range to std.traits module.
Using rvalueOf would introduce unnecessary template instantiation cost. (range traits are frequently used)

DmitryOlshansky · 2014-01-25T21:21:24Z

@9rnsr

I think using rvalueOf in std.range traits is not good. (Note that using lvalueOf is not correct) Because:

Yeah, right lvalueOf would require a copy during T val = lvalueOf!T;

There is already dependency.
std.traits is a frequent dependency and is light-weight compared to others.
https://github.com/9rnsr/phobos/blob/fix_predicates/std/range.d#L293
Yes but how would we then encourage the use of rvalueOf?
I'd agree with .init version if there are some rough numbers suggesting that the impact is large enough to worry.

monarchdodra · 2014-01-25T21:55:59Z

@9rnsr : "Note that using lvalueOf is not correct" : Are you saying that it produces wrong result, or is just not the best thing to use here?

I think this pull is correct, and better than before, so I think it is pull worthy. Bet I still have some trouble accepting that this:

enum bool isOutputRange = is(typeof(
  (inout int = 0)
  {
     R r = R.init;
     E e = E.init;
     put(r, e);
  }));

is better than this:

enum bool isOutputRange = is(typeof(put(lvalueOf!R, lvalueOf!E)));

Questions: Are lvalueOf and rvalueOf really that expensive? They are trivial, have no actual implementation (!), and depend on a single type. As for the dependency thing: everyone depends on traits. I don't think this is a problem.

If we can't use them for this, then they kind of fail for the very thing they were meant to solve...

9rnsr · 2014-01-26T03:50:11Z

"Note that using lvalueOf is not correct" : Are you saying that it produces wrong result, or is just not the best thing to use here?

I meant that: "lvalueOf would require a copy during T val = lvalueOf!T;"

Questions: Are lvalueOf and rvalueOf really that expensive?

Actually we had had same expensive cost around isXXX and XXXTypeOf templates in std.traits. Now the cost is fundamentally reduced by __traits(getAliasThis) feature, but using [lr]valueOf for often used range-traits templates would introduce similar template instantiation cost issue again.

monarchdodra · 2014-01-26T14:15:02Z

I meant that: "lvalueOf would require a copy during T val = lvalueOf!T;"

True, but if you go ahead and use valueOf, you can just use it as-is, you don't need to put it in a variable.

enum bool isOutputRange = is(typeof(
  (inout int = 0)
  {
     alias r = lvalueOf!R;
     alias e = lvalueOf!E;
     put(r, e);
  }));

Or, the 1-liner I showed above: enum bool isOutputRange = is(typeof(put(lvalueOf!R, lvalueOf!E)));

Actually we had had same expensive cost around isXXX and XXXTypeOf templates in std.traits.

I'm not sure these are comparable, since the isXXX templates actually deployed a lot of code to work: If I remember correctly, the compiler had to check in the vicinity of 20 overloads (!) to give a reply. Combined with the fact that there are tons of different isXXX, and it quickly piles up, yes.

lvalueOf and rvalueOf, on the other hand, don't even have a body. They operate on a single type, and don't have any variations. Will using such a template function with no actual body really going to cost us something?

monarchdodra · 2014-02-11T09:28:35Z

@9rnsr : rebase this and I'll pull it. As I said, I don't completely agree with approach, but it is still better than before.

It was a hack for the compiler bugs around T.init. Now the they are properly fixed, so the unused helper function is actually unnecessary.

Use T.init property instead of void initializer, because it will work even if T is const or immutable type. I must change a part of unit test for the bug 6935, because we cannot support ranges which overrides init property.

9rnsr · 2014-02-11T09:37:43Z

OK, rebased commits.

Fix predicate template implementations in std.traits and std.range

monarchdodra · 2014-02-15T07:21:41Z

Merged! Finally :)

Thanks.

9rnsr mentioned this pull request Oct 6, 2012

Issue 8753 - Too aggressive expansion for variables which have void initializer dlang/dmd#1162

Closed

andralex reviewed Oct 6, 2012
View reviewed changes

alexrp reviewed Oct 19, 2012
View reviewed changes

ghost reviewed Mar 9, 2013
View reviewed changes

9rnsr closed this Oct 24, 2013

9rnsr reopened this Nov 4, 2013

9rnsr added 3 commits February 11, 2014 18:35

Remove package function defaultInit

f822e7a

It was a hack for the compiler bugs around T.init. Now the they are properly fixed, so the unused helper function is actually unnecessary.

Add a test case isAssignable!(void[1], void[1])

768907a

monarchdodra added a commit that referenced this pull request Feb 15, 2014

Merge pull request #842 from 9rnsr/fix_predicates

05e611a

Fix predicate template implementations in std.traits and std.range

monarchdodra merged commit 05e611a into dlang:master Feb 15, 2014

9rnsr deleted the fix_predicates branch February 15, 2014 08:09

Uh oh!

Fix predicate template implementations in std.traits and std.range #842

Fix predicate template implementations in std.traits and std.range #842

Uh oh!

Conversation

9rnsr commented Oct 6, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andralex commented Oct 7, 2012

Uh oh!

monarchdodra commented Oct 7, 2012

Uh oh!

andralex commented Oct 14, 2012

Uh oh!

9rnsr commented Oct 14, 2012

Uh oh!

9rnsr commented Oct 14, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexrp commented Oct 19, 2012

Uh oh!

9rnsr commented Dec 5, 2012

Uh oh!

alexrp commented Mar 8, 2013

Uh oh!

9rnsr commented Mar 9, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmdavis commented Mar 9, 2013

Uh oh!

9rnsr commented Mar 9, 2013

Uh oh!

denis-sh commented Oct 2, 2013

Uh oh!

monarchdodra commented Oct 3, 2013

Uh oh!

denis-sh commented Oct 3, 2013

Uh oh!

ghost commented Oct 24, 2013

Uh oh!

monarchdodra commented Oct 24, 2013

Uh oh!

9rnsr commented Nov 4, 2013

Uh oh!

9rnsr commented Nov 4, 2013

Uh oh!

DmitryOlshansky commented Jan 25, 2014

Uh oh!

9rnsr commented Jan 25, 2014

Uh oh!

DmitryOlshansky commented Jan 25, 2014

Uh oh!

monarchdodra commented Jan 25, 2014

Uh oh!

9rnsr commented Jan 26, 2014

Uh oh!

monarchdodra commented Jan 26, 2014

Uh oh!

monarchdodra commented Feb 11, 2014

Uh oh!

9rnsr commented Feb 11, 2014

Uh oh!

monarchdodra commented Feb 15, 2014