Re: Wrong lowering for a[b][c]++

2012-03-23 Thread Andrej Mitrovic
On 3/23/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
 WAT?! What on earth is cast() supposed to mean??

I've no idea. It's probably a front-end bug and the cast forces the
compiler to.. come to its senses?


Re: Wrong lowering for a[b][c]++

2012-03-23 Thread James Miller
On 23 March 2012 19:15, Andrej Mitrovic andrej.mitrov...@gmail.com wrote:
 I've no idea. It's probably a front-end bug and the cast forces the
 compiler to.. come to its senses?

`cast()` is the compiler equivalent to a slap with a wet fish?

--
James Miller


Re: Wrong lowering for a[b][c]++

2012-03-23 Thread Ed McCardell

On 03/23/2012 01:24 AM, H. S. Teoh wrote:

WAT?! What on earth is cast() supposed to mean??


I think it removes one level of const or immutable from the type of its 
argument. Why that helps in this case, I don't know.


--Ed


Re: Wrong lowering for a[b][c]++

2012-03-23 Thread Daniel Murphy
H. S. Teoh hst...@quickfur.ath.cx wrote in message 
news:mailman.1036.1332480215.4860.digitalmar...@puremagic.com...
 On Fri, Mar 23, 2012 at 06:11:05AM +0100, Andrej Mitrovic wrote:
 [...]
 Btw, want to see a magic trick? Put this into your hash:

 this(AA)(AA aa)
 if (std.traits.isAssociativeArray!AA
  is(KeyType!AA == keytype)
  is(ValueType!AA == valuetype))
 {
 foreach (key, val; aa)
 this[key] = val;
 }

 And thn. *drumroll*:

 AA!(string,int) bb = cast()[abc:123];

 badoom-tshhh. LOL!

 WAT?! What on earth is cast() supposed to mean?? That's just screwed
 up. Well anyway, I pushed the change to github, since it at least makes
 literal sliiightly more usable. Have fun! :-)


My guess is that cast forces it to be parsed as an ExpInitializer instead of 
an ArrayInitializer.  Associative array literals as initializers are parsed 
as array initializers then reinterpreted during semantic.

And cast() is like cast(const) or cast(shared). 




Re: Wrong lowering for a[b][c]++

2012-03-23 Thread Timon Gehr

On 03/23/2012 07:15 AM, Andrej Mitrovic wrote:

On 3/23/12, H. S. Teohhst...@quickfur.ath.cx  wrote:

WAT?! What on earth is cast() supposed to mean??


I've no idea. It's probably a front-end bug and the cast forces the
compiler to.. come to its senses?


That part is not a bug, it is specified.

http://dlang.org/expression.html#CastExpression

Casting with no Type or CastQual removes any top level const, 
immutable, shared or inout type modifiers from the type of the 
UnaryExpression.


What is a bug is that array initializers cannot be used to initialize 
structs through associative array alias this.


Maybe you are also missing that this is valid code:

int[] a = [1 : 2, 3 : 4];

This only works for initializers of the form [ ... ]. The cast() removes 
that possibility.


This is why the compiler gets confused.



Re: Wrong lowering for a[b][c]++

2012-03-23 Thread Andrej Mitrovic
On 3/23/12, Timon Gehr timon.g...@gmx.ch wrote:
 Maybe you are also missing that this is valid code:
 int[] a = [1 : 2, 3 : 4];

What is this syntax for and how is it used? It creates '[0, 2, 0, 4]',
which is puzzling to me.


Re: Wrong lowering for a[b][c]++

2012-03-23 Thread Timon Gehr

On 03/23/2012 09:10 PM, Andrej Mitrovic wrote:

On 3/23/12, Timon Gehrtimon.g...@gmx.ch  wrote:

Maybe you are also missing that this is valid code:
int[] a = [1 : 2, 3 : 4];


What is this syntax for and how is it used? It creates '[0, 2, 0, 4]',
which is puzzling to me.


It creates an array from key-value pairs. a[1] will be 2 and a[3] will 
be 4. Unspecified entries are default-initialized. It can be quite 
useful for building lookup tables.


Another reason why array initializers are different from array literals:
struct S{int a,b,c;}
S[] sarr = [{a: 1, b: 2, c: 3}, {a: 4, b: 5, c: 6}];


Re: Wrong lowering for a[b][c]++

2012-03-23 Thread Andrej Mitrovic
On 3/23/12, Timon Gehr timon.g...@gmx.ch wrote:
 It creates an array from key-value pairs. a[1] will be 2 and a[3] will
 be 4. Unspecified entries are default-initialized. It can be quite
 useful for building lookup tables.

Interesting. It's documented under Static Initialization of
Statically Allocated Arrays, but I guess it works for dynamic arrays
too. I could use these for sure. :)

 Another reason why array initializers are different from array literals:
 struct S{int a,b,c;}
 S[] sarr = [{a: 1, b: 2, c: 3}, {a: 4, b: 5, c: 6}];

Yeah I knew about those, although IIRC these might be deprecated?


Re: Wrong lowering for a[b][c]++

2012-03-23 Thread bearophile
Andrej Mitrovic:

 On 3/23/12, Timon Gehr timon.g...@gmx.ch wrote:
  Maybe you are also missing that this is valid code:
  int[] a = [1 : 2, 3 : 4];
 
 What is this syntax for and how is it used? It creates '[0, 2, 0, 4]',
 which is puzzling to me.

See:
http://d.puremagic.com/issues/show_bug.cgi?id=4703

This is why I was unnerved when Walter has recently said that we should reduce 
the amount of breaking changes in D. There are several D problems like that one 
that should be fixed. This is an example of bug report that needs to be 
addressed sooner instead of later.

Bye,
bearophile


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread Andrej Mitrovic
On 3/21/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
 Sorry, I was thinking in terms of my AA implementation which is done
 using a struct with operator overloading. I should've checked what the
 behaviour of the current built-in AAs are before posting. :-P

Well in that case isn't it a necessity to keep compatibility with the
old implementation? Do we really have to break user-code again?


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread Don Clugston

On 21/03/12 21:41, Alvaro wrote:

El 21/03/2012 19:39, Jonathan M Davis escribió:

On Wednesday, March 21, 2012 11:29:14 H. S. Teoh wrote:

A question was asked on the d-learn forum about why this throws a
RangeError:

int[string][int] map;
map[abc][20]++;

This is understandable, since the compiler translates the second line
to:

map.opIndex(abc).opIndexUnary!++(20);

Since map[abc] doesn't exist yet, opIndex throws RangeError before we
ever get to the ++.

I'd like to propose the following fix: if a given chained indexing
expression has any operator applied to its final result (either a unary
operator like ++ or --, or an assignment operator like +=), then instead
of translating previous indexes into opIndex, the compiler should map it
to a new operator overload, say opIndexCreate, which creates the
relevant entry with default value if it doesn't exist yet. That is to
say:

map[abc][20]++;

should be translated to:

map.opIndexCreate(abc).opIndexUnary!++(20);

where opIndexCreate looks something like:

Slot opIndexCreate(Key k) {
Slot *s = findSlot(k);
if (s is null) {
s = createNewSlot(k);
}
return s;
}

Similar changes should be made for expressions like a[b][c][d]=100, or
a[b][c][d]+=100.

In other words, if the tail of a chain of indexing operations maps to
opIndexAssign, opIndexUnary, or opIndexOpAssign, then all preceding
opIndex calls should be converted to opIndexCreate instead.

Comments?


IMHO, it's _horrible_ that C++ creates entries in a std::map when you
ask for
values that aren't there. A RangeError is _exactly_ what should be
happening
here. There's no element to increment, because it hasn't been added
yet. I
think that the current behavior is very much what the behavior
_should_ be.

- Jonathan M Davis


I partially disagree. I think items should be added if we try to *write*
to them, but not when we *read* them (lvalue vs rvalue). The problem is
that it's hard to distinguish those cases in C++ without an intermediate
class that makes its use uglier. So:

int[int] a;
a[3] = 1; // OK, add key 3
b = a[5]; // error, add nothing

And, compound assignment and increment/decrement are forms of *writing*.


Yes, but they read before they write.


So, they should trigger an element creation with the .init value.

a[5]++; // create a[5] = int.init; and then increment




Re: Wrong lowering for a[b][c]++

2012-03-22 Thread Jesse Phillips

On Wednesday, 21 March 2012 at 20:41:17 UTC, Alvaro wrote:

I partially disagree. I think items should be added if we try 
to *write* to them, but not when we *read* them (lvalue vs 
rvalue). The problem is that it's hard to distinguish those 
cases in C++ without an intermediate class that makes its use 
uglier. So:


int[int] a;
a[3] = 1; // OK, add key 3
b = a[5]; // error, add nothing

And, compound assignment and increment/decrement are forms of 
*writing*. So, they should trigger an element creation with the 
.init value.


a[5]++; // create a[5] = int.init; and then increment


I have used similar before, it is nicer than

if(5 !in a)
a[5] = 1;
else
a[5]++;

but after taking a look at reduce.

double[int] a;

What is the result of your code on 'a' now? double.init is NAN.


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread Andrej Mitrovic
On 3/22/12, Jesse Phillips jessekphillip...@gmail.com wrote:
 double[int] a;
 What is the result of your code on 'a' now? double.init is NAN.

Hmm this is interesting. With 2.058:

double[int] a;
a[0]++;
writeln(a[0]);  // prints 1

double b;
b++;
writeln(b);  // prints nan


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread H. S. Teoh
On Thu, Mar 22, 2012 at 11:01:35AM +0100, Andrej Mitrovic wrote:
 On 3/21/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
  Sorry, I was thinking in terms of my AA implementation which is done
  using a struct with operator overloading. I should've checked what the
  behaviour of the current built-in AAs are before posting. :-P
 
 Well in that case isn't it a necessity to keep compatibility with the
 old implementation? Do we really have to break user-code again?

Yes we have to keep compatibility. But as some have pointed out, this
works:

int[string][int] map;
map[20][abc]++;   // sets map[20][abc] to 1

Looking at druntime code, I see that in aaA.d there's _aaGetX (which
creates a new entry if it doesn't already exist) and _aaGetRvalueX
(doesn't create new entry).  Which look like the equivalents of
opIndexCreate and opIndex.

The problem is that the language doesn't currently support the former in
user-defined structs; so this *will* break compatibility.


T

-- 
Real Programmers use cat  a.out.


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread H. S. Teoh
On Thu, Mar 22, 2012 at 03:26:00PM +0100, Andrej Mitrovic wrote:
 On 3/22/12, Jesse Phillips jessekphillip...@gmail.com wrote:
  double[int] a;
  What is the result of your code on 'a' now? double.init is NAN.
 
 Hmm this is interesting. With 2.058:
 
 double[int] a;
 a[0]++;
 writeln(a[0]);  // prints 1
 
 double b;
 b++;
 writeln(b);  // prints nan

This is because in aaA.d _aaGetX creates a new entry if one isn't found,
but because it has no direct access to value types (only has typeinfo),
it doesn't know what value .init has. It sets the value to binary zero
by default.

(This is one of the places where you *really* want the AA implementation
to be in object_.d, where it can actually say Value.init instead of
hacking it with binary zero, which is wrong in this case but pretty much
impossible to fix.)


T

-- 
If it tastes good, it's probably bad for you.


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread Andrej Mitrovic
On 3/22/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
 This is because in aaA.d _aaGetX creates a new entry if one isn't found,
 but because it has no direct access to value types (only has typeinfo),
 it doesn't know what value .init has. It sets the value to binary zero
 by default.

Damn, hashes really are a terrible hack. I'm really looking forward to
your new implementation.


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread Andrei Alexandrescu

On 3/22/12 9:32 AM, H. S. Teoh wrote:

On Thu, Mar 22, 2012 at 11:01:35AM +0100, Andrej Mitrovic wrote:

On 3/21/12, H. S. Teohhst...@quickfur.ath.cx  wrote:

Sorry, I was thinking in terms of my AA implementation which is done
using a struct with operator overloading. I should've checked what the
behaviour of the current built-in AAs are before posting. :-P


Well in that case isn't it a necessity to keep compatibility with the
old implementation? Do we really have to break user-code again?


Yes we have to keep compatibility. But as some have pointed out, this
works:

int[string][int] map;
map[20][abc]++; // sets map[20][abc] to 1

Looking at druntime code, I see that in aaA.d there's _aaGetX (which
creates a new entry if it doesn't already exist) and _aaGetRvalueX
(doesn't create new entry).  Which look like the equivalents of
opIndexCreate and opIndex.

The problem is that the language doesn't currently support the former in
user-defined structs; so this *will* break compatibility.


This is a bug in the language that requires fixing.

Andrei




Re: Wrong lowering for a[b][c]++

2012-03-22 Thread bearophile

Andrei Alexandrescu:


This is a bug in the language that requires fixing.


I agree. Is someone willing to write a good enhancement request
about it?

Bye,
bearophile


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread H. S. Teoh
On Thu, Mar 22, 2012 at 03:48:24PM +0100, Andrej Mitrovic wrote:
 On 3/22/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
  This is because in aaA.d _aaGetX creates a new entry if one isn't found,
  but because it has no direct access to value types (only has typeinfo),
  it doesn't know what value .init has. It sets the value to binary zero
  by default.
 
 Damn, hashes really are a terrible hack. I'm really looking forward to
 your new implementation.

You can check out the current code here:

https://github.com/quickfur/New-AA-implementation

You can just import newAA. Obviously it's not integrated with
druntime/dmd yet, so it doesn't have the nice V[K] or literal syntax.
But you can use it as AssociativeArray!(K,V). There's also a fromLiteral
static method that you can use to simulate literals (I'm considering
making it a module function instead of a static method, though).

It should work like the current implementation, for the most part,
except for some outstanding issues I'm working on.

Actually, it would be nice if people gave me some feedback on how well
the current implementation works. :-) I may not be aware of all the
issues that need fixing.


T

-- 
Life is complex. It consists of real and imaginary parts. -- YHL


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread Andrej Mitrovic
On 3/22/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
 You can check out the current code here:

   https://github.com/quickfur/New-AA-implementation

I need Git Head (2.059) for this, right? Otherwise I get:
newAA.d(426): Error: inout on parameter means inout must be on return type
as well (if from D1 code, replace with 'ref')


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread H. S. Teoh
On Thu, Mar 22, 2012 at 05:52:54PM +0100, Andrej Mitrovic wrote:
 On 3/22/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
  You can check out the current code here:
 
  https://github.com/quickfur/New-AA-implementation
 
 I need Git Head (2.059) for this, right? Otherwise I get:
 newAA.d(426): Error: inout on parameter means inout must be on return
 type as well (if from D1 code, replace with 'ref')

Probably. I've been developing on 2.059 so I didn't realize there are
incompatibilities with earlier versions.


T

-- 
People tell me that I'm paranoid, but they're just out to get me.


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread Martin Nowak
On Thu, 22 Mar 2012 15:35:06 +0100, H. S. Teoh hst...@quickfur.ath.cx  
wrote:



On Thu, Mar 22, 2012 at 03:26:00PM +0100, Andrej Mitrovic wrote:

On 3/22/12, Jesse Phillips jessekphillip...@gmail.com wrote:
 double[int] a;
 What is the result of your code on 'a' now? double.init is NAN.

Hmm this is interesting. With 2.058:

double[int] a;
a[0]++;
writeln(a[0]);  // prints 1

double b;
b++;
writeln(b);  // prints nan


This is because in aaA.d _aaGetX creates a new entry if one isn't found,
but because it has no direct access to value types (only has typeinfo),
it doesn't know what value .init has. It sets the value to binary zero
by default.

TypeInfos have a ubyte[] array with the needed data.
It's badly named though.

rdmd --eval=writeln(typeid(double).init)

But the AA doesn't have access to the typeinfo of the value anyhow.


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread Andrej Mitrovic
On 3/22/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
 Probably. I've been developing on 2.059 so I didn't realize there are
 incompatibilities with earlier versions.

Your test files work ok. I'll test it on my own projects as soon as I
get rid of a few compilation errors (all of a sudden I get 900 error
lines for failed template instantiations, LOL).


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread H. S. Teoh
On Thu, Mar 22, 2012 at 06:43:29PM +0100, Andrej Mitrovic wrote:
 On 3/22/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
  Probably. I've been developing on 2.059 so I didn't realize there
  are incompatibilities with earlier versions.
 
 Your test files work ok. I'll test it on my own projects as soon as I
 get rid of a few compilation errors (all of a sudden I get 900 error
 lines for failed template instantiations, LOL).

Yeah that's probably caused by a recent dmd change to soldier on after a
template instantiation error. :-)

Some template instantiation problems are my fault, though. There are
still some AA's that don't instantiate correctly. Please let me know any
that you find, 'cos they need to be fixed.


T

-- 
The peace of mind---from knowing that viruses which exploit Microsoft system 
vulnerabilities cannot touch Linux---is priceless. -- Frustrated system 
administrator.


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread Andrej Mitrovic
On 3/22/12, Andrej Mitrovic andrej.mitrov...@gmail.com wrote:
 Your test files work ok.

Well unfortunately isAssociativeArray from std.traits fails on your
hashes (reasonable, since it calls __traits internally and has no way
of knowing about external hashes).

I also can't use your hashes with some helper functions that I use:

template ValueType(V : V[K], K) {
alias V ValueType;
}

The function that needs the above is:

void addKey(AA, Key)(ref AA hash, Key key)
  if (isAssociativeArray!AA  is(KeyType!AA == Key))
{
hash[key] = (ValueType!AA).init;
}

I can temporarily remove those constraints, but how do I extract the
value type of your hash?


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread Andrej Mitrovic
On 3/22/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
 Yeah that's probably caused by a recent dmd change to soldier on after a
 template instantiation error. :-)

Hopefully that gets fixed by the next release. Otherwise we can expect
an angry dev creating a D FQA site, heheh. :P


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread H. S. Teoh
On Thu, Mar 22, 2012 at 07:04:09PM +0100, Andrej Mitrovic wrote:
 On 3/22/12, Andrej Mitrovic andrej.mitrov...@gmail.com wrote:
  Your test files work ok.
 
 Well unfortunately isAssociativeArray from std.traits fails on your
 hashes (reasonable, since it calls __traits internally and has no way
 of knowing about external hashes).
 
 I also can't use your hashes with some helper functions that I use:
 
 template ValueType(V : V[K], K) {
 alias V ValueType;
 }
 
 The function that needs the above is:
 
 void addKey(AA, Key)(ref AA hash, Key key)
   if (isAssociativeArray!AA  is(KeyType!AA == Key))
 {
 hash[key] = (ValueType!AA).init;
 }
 
 I can temporarily remove those constraints, but how do I extract the
 value type of your hash?

Hmm. Perhaps some aliases are in order?

struct AssociativeArray(K,V) {
...
alias Key keytype;
alias Value valuetype;
...
}

Presumably this won't be an issue once it's integrated with dmd. But
these aliases might be useful for other things, too.


T

-- 
You know, maybe we don't *need* enemies. Yeah, best friends are about all I 
can take. -- Calvin  Hobbes


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread H. S. Teoh
On Thu, Mar 22, 2012 at 11:21:43AM -0700, H. S. Teoh wrote:
[...]
 Hmm. Perhaps some aliases are in order?
 
   struct AssociativeArray(K,V) {
   ...
   alias Key keytype;
   alias Value valuetype;
   ...
   }
[...]

OK, I've added these aliases and pushed to github. I think they're a
useful thing to have. If not, it's easy to remove before we integrate
with druntime. But at least in the meantime it will make these AA's
easier to work with.


T

-- 
Bomb technician: If I'm running, try to keep up.


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread Andrej Mitrovic
On 3/22/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
 OK, I've added these aliases and pushed to github. I think they're a
 useful thing to have. If not, it's easy to remove before we integrate
 with druntime. But at least in the meantime it will make these AA's
 easier to work with.

Could you also add an enum to the template like so:?

struct AssociativeArray(Key,Value)
{
enum isAssociativeArray = true;
...
}

I've realized that by just removing constraints I'm ending up with a
lot of conflicts (doh!), but I can implement a custom template that
checks for this enum field instead.


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread Andrej Mitrovic
 On 3/22/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
 snip

Hmm, what about (temporary) compatibility with hash literals? You
could add the following to the newAA module:

static import std.traits;
template KeyType(V : V[K], K)
{
alias K KeyType;
}

template ValueType(V : V[K], K)
{
alias V ValueType;
}

and in the AssociativeArray struct:
void opAssign(AA)(AA aa)
if (std.traits.isAssociativeArray!AA
 is(KeyType!AA == keytype)
 is(ValueType!AA == valuetype))
{
foreach (key, val; aa)
this[key] = val;
}

Then this will work:

AA!(int, string) intToStr;
intToStr = [1:foo];
assert(intToStr[1] == foo);

Otherwise it will be pretty hard to test the new hashes without
changing a significant amount of code.


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread Don Clugston

On 22/03/12 18:43, Andrej Mitrovic wrote:

On 3/22/12, H. S. Teohhst...@quickfur.ath.cx  wrote:

Probably. I've been developing on 2.059 so I didn't realize there are
incompatibilities with earlier versions.


Your test files work ok. I'll test it on my own projects as soon as I
get rid of a few compilation errors (all of a sudden I get 900 error
lines for failed template instantiations, LOL).


Please report them as bugs. I know of one situation where that happens, 
but I'm sure there are more.


I want to get to the point where there is exactly one error message for 
each bug in your code. Previously the compiler was cheating by calling 
exit() as soon as it got ANY template error, but the cheating also 
caused subtle bugs, because it can't abort if errors are gagged.


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread H. S. Teoh
On Thu, Mar 22, 2012 at 07:55:09PM +0100, Andrej Mitrovic wrote:
[...]
 Hmm, what about (temporary) compatibility with hash literals? You
 could add the following to the newAA module:
 
 static import std.traits;
 template KeyType(V : V[K], K)
 {
 alias K KeyType;
 }
 
 template ValueType(V : V[K], K)
 {
 alias V ValueType;
 }

Done, pushed to github.


 and in the AssociativeArray struct:
 void opAssign(AA)(AA aa)
 if (std.traits.isAssociativeArray!AA
  is(KeyType!AA == keytype)
  is(ValueType!AA == valuetype))
 {
 foreach (key, val; aa)
 this[key] = val;
 }

Done, though this required another opAssign() to replace the default one
that got suppressed by this template. Which required some nasty const
casts to work properly, but at least D lets you do that when you need
to. :-)


 Then this will work:
 
 AA!(int, string) intToStr;
 intToStr = [1:foo];
 assert(intToStr[1] == foo);
 
 Otherwise it will be pretty hard to test the new hashes without
 changing a significant amount of code.

True. I still can't get this to work though:

AA!(string,int) aa = [abc:123];

Maybe this needs a ctor?


T

-- 
Almost all proofs have bugs, but almost all theorems are true. -- Paul Pedersen


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread Andrej Mitrovic
On 3/22/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
 Maybe this needs a ctor?

Yeah probably. I really don't know when opAssign or the ctor are
called (especially with more complex types with alias this). Maybe
someone knows this or it's in the spec but I haven't read about it.


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread H. S. Teoh
On Thu, Mar 22, 2012 at 04:31:00PM +0100, bearophile wrote:
 Andrei Alexandrescu:
 
 This is a bug in the language that requires fixing.
 
 I agree. Is someone willing to write a good enhancement request
 about it?
[...]

Done:

http://d.puremagic.com/issues/show_bug.cgi?id=7753

Please comment/vote. ;-) Thanks!


T

-- 
Winners never quit, quitters never win. But those who never quit AND never win 
are idiots.


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread H. S. Teoh
On Thu, Mar 22, 2012 at 10:25:04PM +0100, Andrej Mitrovic wrote:
 On 3/22/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
  Maybe this needs a ctor?
 
 Yeah probably. I really don't know when opAssign or the ctor are
 called (especially with more complex types with alias this). Maybe
 someone knows this or it's in the spec but I haven't read about it.

Gah. I tried adding a ctor but it still doesn't work. Compiler
complains:

Error: Integer constant expression expected instead of abc

for AA!(string,int) bb = [abc:123];

I'm guessing the compiler thinks the literal is an array literal, or
maybe something went awry with the internal AA hacks that it currently
has. So we can't have this nice syntax until druntime/dmd integration.
:-(


T

-- 
Many open minds should be closed for repairs. -- K5 user


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread H. S. Teoh
On Thu, Mar 22, 2012 at 09:29:36PM -0700, H. S. Teoh wrote:
 On Thu, Mar 22, 2012 at 10:25:04PM +0100, Andrej Mitrovic wrote:
  On 3/22/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
   Maybe this needs a ctor?
  
  Yeah probably. I really don't know when opAssign or the ctor are
  called (especially with more complex types with alias this). Maybe
  someone knows this or it's in the spec but I haven't read about it.
 
 Gah. I tried adding a ctor but it still doesn't work. Compiler
 complains:
 
   Error: Integer constant expression expected instead of abc
 
 for AA!(string,int) bb = [abc:123];
 
 I'm guessing the compiler thinks the literal is an array literal, or
 maybe something went awry with the internal AA hacks that it currently
 has. So we can't have this nice syntax until druntime/dmd integration.
[...]

Seems to be related to this bug:

http://d.puremagic.com/issues/show_bug.cgi?id=6469


T

-- 
The right half of the brain controls the left half of the body. This means that 
only left-handed people are in their right mind. -- Manoj Srivastava


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread Andrej Mitrovic
On 3/23/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
 I'm guessing the compiler thinks the literal is an array literal, or
 maybe something went awry with the internal AA hacks that it currently
 has.

struct Foo
{
string[int] aa;
alias aa this;
}

void main()
{
Foo x = [1 : 4];
}

test.d(22): Error: cannot use array to initialize Foo

Borken to the bone. I don't know whether there's a bug report open on this.

Btw, want to see a magic trick? Put this into your hash:

this(AA)(AA aa)
if (std.traits.isAssociativeArray!AA
 is(KeyType!AA == keytype)
 is(ValueType!AA == valuetype))
{
foreach (key, val; aa)
this[key] = val;
}

And thn. *drumroll*:

AA!(string,int) bb = cast()[abc:123];

badoom-tshhh. LOL!


Re: Wrong lowering for a[b][c]++

2012-03-22 Thread H. S. Teoh
On Fri, Mar 23, 2012 at 06:11:05AM +0100, Andrej Mitrovic wrote:
[...]
 Btw, want to see a magic trick? Put this into your hash:
 
 this(AA)(AA aa)
 if (std.traits.isAssociativeArray!AA
  is(KeyType!AA == keytype)
  is(ValueType!AA == valuetype))
 {
 foreach (key, val; aa)
 this[key] = val;
 }
 
 And thn. *drumroll*:
 
 AA!(string,int) bb = cast()[abc:123];
 
 badoom-tshhh. LOL!

WAT?! What on earth is cast() supposed to mean?? That's just screwed
up. Well anyway, I pushed the change to github, since it at least makes
literal sliiightly more usable. Have fun! :-)


T

-- 
What's a hot crossed bun? An angry rabbit.


Wrong lowering for a[b][c]++

2012-03-21 Thread H. S. Teoh
A question was asked on the d-learn forum about why this throws a
RangeError:

int[string][int] map;
map[abc][20]++;

This is understandable, since the compiler translates the second line to:

map.opIndex(abc).opIndexUnary!++(20);

Since map[abc] doesn't exist yet, opIndex throws RangeError before we
ever get to the ++.

I'd like to propose the following fix: if a given chained indexing
expression has any operator applied to its final result (either a unary
operator like ++ or --, or an assignment operator like +=), then instead
of translating previous indexes into opIndex, the compiler should map it
to a new operator overload, say opIndexCreate, which creates the
relevant entry with default value if it doesn't exist yet. That is to
say:

map[abc][20]++;

should be translated to:

map.opIndexCreate(abc).opIndexUnary!++(20);

where opIndexCreate looks something like:

Slot opIndexCreate(Key k) {
Slot *s = findSlot(k);
if (s is null) {
s = createNewSlot(k);
}
return s;
}

Similar changes should be made for expressions like a[b][c][d]=100, or
a[b][c][d]+=100.

In other words, if the tail of a chain of indexing operations maps to
opIndexAssign, opIndexUnary, or opIndexOpAssign, then all preceding
opIndex calls should be converted to opIndexCreate instead.

Comments?


T

-- 
One Word to write them all, One Access to find them, One Excel to count them 
all, And thus to Windows bind them. -- Mike Champion


Re: Wrong lowering for a[b][c]++

2012-03-21 Thread Jonathan M Davis
On Wednesday, March 21, 2012 11:29:14 H. S. Teoh wrote:
 A question was asked on the d-learn forum about why this throws a
 RangeError:
 
 int[string][int] map;
 map[abc][20]++;
 
 This is understandable, since the compiler translates the second line to:
 
 map.opIndex(abc).opIndexUnary!++(20);
 
 Since map[abc] doesn't exist yet, opIndex throws RangeError before we
 ever get to the ++.
 
 I'd like to propose the following fix: if a given chained indexing
 expression has any operator applied to its final result (either a unary
 operator like ++ or --, or an assignment operator like +=), then instead
 of translating previous indexes into opIndex, the compiler should map it
 to a new operator overload, say opIndexCreate, which creates the
 relevant entry with default value if it doesn't exist yet. That is to
 say:
 
 map[abc][20]++;
 
 should be translated to:
 
 map.opIndexCreate(abc).opIndexUnary!++(20);
 
 where opIndexCreate looks something like:
 
 Slot opIndexCreate(Key k) {
 Slot *s = findSlot(k);
 if (s is null) {
 s = createNewSlot(k);
 }
 return s;
 }
 
 Similar changes should be made for expressions like a[b][c][d]=100, or
 a[b][c][d]+=100.
 
 In other words, if the tail of a chain of indexing operations maps to
 opIndexAssign, opIndexUnary, or opIndexOpAssign, then all preceding
 opIndex calls should be converted to opIndexCreate instead.
 
 Comments?

IMHO, it's _horrible_ that C++ creates entries in a std::map when you ask for 
values that aren't there. A RangeError is _exactly_ what should be happening 
here. There's no element to increment, because it hasn't been added yet. I 
think that the current behavior is very much what the behavior _should_ be.

- Jonathan M Davis


Re: Wrong lowering for a[b][c]++

2012-03-21 Thread David Nadlinger

On Wednesday, 21 March 2012 at 18:27:30 UTC, H. S. Teoh wrote:
A question was asked on the d-learn forum about why this throws 
a

RangeError:

int[string][int] map;
map[abc][20]++;


Wait a second – aren't AAs _supposed_ to throw if accessing a 
key that doesn't exist yet? To be able to increment something, 
there already has to be a value to start from…


David


Re: Wrong lowering for a[b][c]++

2012-03-21 Thread Andrej Mitrovic
On 3/21/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
 Comments?

Initially I was against hash[key]++ working on non-existent keys, but
I've changed my mind about this. However it still throws me off that
hash[key1][key2]++ doesn't work if key1 doesn't exist.

So you get a +1 vote from me.


Re: Wrong lowering for a[b][c]++

2012-03-21 Thread Andrej Mitrovic
On 3/21/12, Jonathan M Davis jmdavisp...@gmx.com wrote:
 I think that the current behavior is very much what the behavior _should_ be.

But the current behavior is already at odds. This already works:

int[string] foo;
foo[bar]++;


Re: Wrong lowering for a[b][c]++

2012-03-21 Thread H. S. Teoh
On Wed, Mar 21, 2012 at 02:39:04PM -0400, Jonathan M Davis wrote:
[...]
 IMHO, it's _horrible_ that C++ creates entries in a std::map when you
 ask for values that aren't there. A RangeError is _exactly_ what
 should be happening here. There's no element to increment, because it
 hasn't been added yet. I think that the current behavior is very much
 what the behavior _should_ be.
[...]

Then what about:

int[string][int] map;
map[abc][30] = 123;

?

Here, you clearly intend to create a new entry, but currently this
doesn't work. It will throw a RangeError when it tries to look up abc.
You need the ugly workaround:

int[string][int] map;
map[abc] = int[int].init;
map[abc][30] = 123;


T

-- 
What doesn't kill me makes me stranger.


Re: Wrong lowering for a[b][c]++

2012-03-21 Thread Andrej Mitrovic
On 3/21/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
   int[string][int] map;
   map[abc] = int[int].init;
   map[abc][30] = 123;

I think you meant:

int[int][string] map;
map[abc] = (int[int]).init;
map[abc][30] = 123;

You can however init with null if the value is a hash:

int[int][string] map;
map[abc] = null;
map[abc][30] = 123;

But otherwise I agree it would be nice if there was a shortcut for this.


Re: Wrong lowering for a[b][c]++

2012-03-21 Thread H. S. Teoh
On Wed, Mar 21, 2012 at 07:29:44PM +0100, David Nadlinger wrote:
 On Wednesday, 21 March 2012 at 18:27:30 UTC, H. S. Teoh wrote:
 A question was asked on the d-learn forum about why this throws a
 RangeError:
 
  int[string][int] map;
  map[abc][20]++;
 
 Wait a second – aren't AAs _supposed_ to throw if accessing a key
 that doesn't exist yet? To be able to increment something, there
 already has to be a value to start from…
[...]

If it doesn't exist, it should default to typeof(value).init, IMO.

But perhaps that was the wrong example to use. What if you wanted to do
this:

map[abc][20] = 1;

Currently this doesn't work. You have to explicitly create map[abc]
first.


T

-- 
Time flies like an arrow. Fruit flies like a banana.


Re: Wrong lowering for a[b][c]++

2012-03-21 Thread Ali Çehreli

On 03/21/2012 11:29 AM, H. S. Teoh wrote:
 A question was asked on the d-learn forum about why this throws a
 RangeError:

int[string][int] map;
map[abc][20]++;

Hey! That syntax is following the broken syntax of C and C++. ;)

This works:

import std.stdio;

void main()
{
int[string][int] map;
map[20][abc]++;

writeln(map);
}

The output:

[20:[abc:1]]

Ali



Re: Wrong lowering for a[b][c]++

2012-03-21 Thread Andrej Mitrovic
On 3/21/12, Ali Çehreli acehr...@yahoo.com wrote:
 This works:

For some reason thought it didn't work without double-checking. Heh. :p


Re: Wrong lowering for a[b][c]++

2012-03-21 Thread H. S. Teoh
On Wed, Mar 21, 2012 at 07:56:47PM +0100, Andrej Mitrovic wrote:
 On 3/21/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
  int[string][int] map;
  map[abc] = int[int].init;
  map[abc][30] = 123;
 
 I think you meant:
 
 int[int][string] map;
 map[abc] = (int[int]).init;
 map[abc][30] = 123;

Yes, I did. :-)


 You can however init with null if the value is a hash:
 
 int[int][string] map;
 map[abc] = null;
 map[abc][30] = 123;
 
 But otherwise I agree it would be nice if there was a shortcut for this.

Without this shortcut, it's a complete pain to deal with deeply-nested
AA's:

int[int][int][string] map;

void inc_frequency(string entry, int x, int y) {
if (entry in map) {
if (x in map[entry]) {
if (y in map[entry][x]) {
map[entry][x][y]++;
} else {
map[entry][x][y] = 1;
}
} else {
map[entry][x] = null;
map[entry][x][y] = 1;
}
} else {
map[entry] = null;
map[entry][x] = null;
map[entry][x][y] = 1;
}
}

whereas if this was supported, the code would simply be:

void inc_frequency(string entry, int xcoor, int ycoor) {
map[entry][x][y]++;
}

(And the above example is only 3 levels deep... it gets exponentially
worse with every level of nesting.)


T

-- 
Mediocrity has been pushed to extremes.


Re: Wrong lowering for a[b][c]++

2012-03-21 Thread Jesse Phillips
H. S. is making a Library replacement AA and is what his example 
is concerned with.


You are correct, it seems his example has it out of order, the 
example on Learn was


int[100][string] map;
...

On Wednesday, 21 March 2012 at 18:58:34 UTC, Ali Çehreli wrote:

This works:

import std.stdio;

void main()
{
int[string][int] map;
map[20][abc]++;

writeln(map);
}

The output:

[20:[abc:1]]

Ali





Re: Wrong lowering for a[b][c]++

2012-03-21 Thread Steven Schveighoffer
On Wed, 21 Mar 2012 14:29:14 -0400, H. S. Teoh hst...@quickfur.ath.cx  
wrote:



A question was asked on the d-learn forum about why this throws a
RangeError:

int[string][int] map;
map[abc][20]++;

This is understandable, since the compiler translates the second line to:

map.opIndex(abc).opIndexUnary!++(20);

Since map[abc] doesn't exist yet, opIndex throws RangeError before we
ever get to the ++.


What about a create accessor of the map which is the same as accessing the  
map, except it ensures T.init is used at each level?


That is:

map.create[abc][20]++;

Whether you want default-created elements or not is an important design  
decision that should not be up to the container.


-Steve


Re: Wrong lowering for a[b][c]++

2012-03-21 Thread Andrej Mitrovic
On 3/21/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
 whereas if this was supported, the code would simply be:

   void inc_frequency(string entry, int xcoor, int ycoor) {
   map[entry][x][y]++;
   }

Wait a minute. Are we missing something? This does work in 2.058:

void inc_frequency(string entry, int x, int y)
{
int[int][int][string] map;
map[entry][x][y]++;
}

void main() {
inc_frequency(bla, 10, 20);
}

I'm now totally confused. :x


Re: Wrong lowering for a[b][c]++

2012-03-21 Thread Alvaro

El 21/03/2012 19:29, H. S. Teoh escribió:

A question was asked on the d-learn forum about why this throws a
RangeError:

int[string][int] map;
map[abc][20]++;



IIRC, that worked fine for me a few weeks ago. And I found it to be just 
great that it did. I'll have to re-check but I remember it like this.


I had to produce a new tabular dataset from an original by accumulating 
elements (rows) sharing some attribute values.


The idea was to read that line by line, split into attributes and do:

 // this will hold the counts:

 int[int][int][int][string] count;

 // for each row, split field1 .. field5 (field4 is a string)

 count[field1][field2][field3][field4] += field5;


And that just worked IIRC, without initializing anything!

At the end, to iterate all the hyper map and write it out, we used 
nested foreachs, something like:


foreach(field1, item1; count)
foreach(field2, item2; item1)
foreach(field3, item3; item2)
foreach(field4, field5; item3)
writeln(field1, field2, field3, field4, field5);

I'm trying to remember if we had to do something special to make it work...



Re: Wrong lowering for a[b][c]++

2012-03-21 Thread Alvaro

El 21/03/2012 19:39, Jonathan M Davis escribió:

On Wednesday, March 21, 2012 11:29:14 H. S. Teoh wrote:

A question was asked on the d-learn forum about why this throws a
RangeError:

int[string][int] map;
map[abc][20]++;

This is understandable, since the compiler translates the second line to:

map.opIndex(abc).opIndexUnary!++(20);

Since map[abc] doesn't exist yet, opIndex throws RangeError before we
ever get to the ++.

I'd like to propose the following fix: if a given chained indexing
expression has any operator applied to its final result (either a unary
operator like ++ or --, or an assignment operator like +=), then instead
of translating previous indexes into opIndex, the compiler should map it
to a new operator overload, say opIndexCreate, which creates the
relevant entry with default value if it doesn't exist yet. That is to
say:

map[abc][20]++;

should be translated to:

map.opIndexCreate(abc).opIndexUnary!++(20);

where opIndexCreate looks something like:

Slot opIndexCreate(Key k) {
Slot *s = findSlot(k);
if (s is null) {
s = createNewSlot(k);
}
return s;
}

Similar changes should be made for expressions like a[b][c][d]=100, or
a[b][c][d]+=100.

In other words, if the tail of a chain of indexing operations maps to
opIndexAssign, opIndexUnary, or opIndexOpAssign, then all preceding
opIndex calls should be converted to opIndexCreate instead.

Comments?


IMHO, it's _horrible_ that C++ creates entries in a std::map when you ask for
values that aren't there. A RangeError is _exactly_ what should be happening
here. There's no element to increment, because it hasn't been added yet. I
think that the current behavior is very much what the behavior _should_ be.

- Jonathan M Davis


I partially disagree. I think items should be added if we try to *write* 
to them, but not when we *read* them (lvalue vs rvalue). The problem is 
that it's hard to distinguish those cases in C++ without an intermediate 
class that makes its use uglier. So:


int[int] a;
a[3] = 1; // OK, add key 3
b = a[5]; // error, add nothing

And, compound assignment and increment/decrement are forms of *writing*. 
So, they should trigger an element creation with the .init value.


a[5]++; // create a[5] = int.init; and then increment


Re: Wrong lowering for a[b][c]++

2012-03-21 Thread Jonathan M Davis
On Wednesday, March 21, 2012 12:00:36 H. S. Teoh wrote:
 On Wed, Mar 21, 2012 at 07:29:44PM +0100, David Nadlinger wrote:
  On Wednesday, 21 March 2012 at 18:27:30 UTC, H. S. Teoh wrote:
  A question was asked on the d-learn forum about why this throws a
  
  RangeError:
   int[string][int] map;
   map[abc][20]++;
  
  Wait a second – aren't AAs _supposed_ to throw if accessing a key
  that doesn't exist yet? To be able to increment something, there
  already has to be a value to start from…
 
 [...]
 
 If it doesn't exist, it should default to typeof(value).init, IMO.
 
 But perhaps that was the wrong example to use. What if you wanted to do
 this:
 
 map[abc][20] = 1;
 
 Currently this doesn't work. You have to explicitly create map[abc]
 first.

Which I think makes perfect sense. I think that implicitly creating elements 
is evil. It's already bad enough IMHO that the language implicitly creates the 
AA itself (especially when you consider stuff like the bugs that occur when you 
pass an AA to a function expecting the elements that it adds to it to be in 
the AA afterwards - it works with an already initialized AA but not a null 
one).

I think that Steven's suggestion of a function for this makes far more sense.

- Jonathan M Davis


Re: Wrong lowering for a[b][c]++

2012-03-21 Thread H. S. Teoh
On Wed, Mar 21, 2012 at 09:25:04PM +0100, Andrej Mitrovic wrote:
 On 3/21/12, H. S. Teoh hst...@quickfur.ath.cx wrote:
  whereas if this was supported, the code would simply be:
 
  void inc_frequency(string entry, int xcoor, int ycoor) {
  map[entry][x][y]++;
  }
 
 Wait a minute. Are we missing something? This does work in 2.058:
 
 void inc_frequency(string entry, int x, int y)
 {
 int[int][int][string] map;
 map[entry][x][y]++;
 }
 
 void main() {
 inc_frequency(bla, 10, 20);
 }
 
 I'm now totally confused. :x

Sorry, I was thinking in terms of my AA implementation which is done
using a struct with operator overloading. I should've checked what the
behaviour of the current built-in AAs are before posting. :-P


T

-- 
Almost all proofs have bugs, but almost all theorems are true. -- Paul Pedersen


Re: Wrong lowering for a[b][c]++

2012-03-21 Thread H. S. Teoh
On Wed, Mar 21, 2012 at 05:21:22PM -0400, Jonathan M Davis wrote:
 On Wednesday, March 21, 2012 12:00:36 H. S. Teoh wrote:
[...]
  But perhaps that was the wrong example to use. What if you wanted to
  do this:
  
  map[abc][20] = 1;
  
  Currently this doesn't work. You have to explicitly create
  map[abc] first.
 
 Which I think makes perfect sense. I think that implicitly creating
 elements is evil.
[...]

I disagree. If you wrote:

if (map[abc][2] == 1)

then I agree that this should throw a RangeError.

But in the case of writing map[abc][20]=1, you're explicitly saying
assign 1 to map[abc][20]. This means the user *wants* to create a
new entry in the AA. So it should create the intermediate entry
necessary as well.

Otherwise, by your reasoning, this should fail too:

int[string] aa;
aa[abc] = 1;  // should throw RangeError since entry doesn't exist

which I think reduces the usability of the indexing syntax.

Having the opIndexAssign syntax work only when there's a single level of
indexing seems to me completely arbitrary.

Having said that, though, I'm a bit on the fence about making ++ or --
work in this case too. Those cases may be arguable because you need
something to exist before you can operate on it, so it may be OK to
throw in those cases. However, in the straight assignment case, I
definitely contend that it needs to work.


T

-- 
BREAKFAST.COM halted...Cereal Port Not Responding. -- YHL


Re: Wrong lowering for a[b][c]++

2012-03-21 Thread Andrei Alexandrescu

On 3/21/12 5:44 PM, H. S. Teoh wrote:

Sorry, I was thinking in terms of my AA implementation which is done
using a struct with operator overloading. I should've checked what the
behaviour of the current built-in AAs are before posting. :-P


The request for autovivification (believe it or not, that's how Perl 
calls it (http://en.wikipedia.org/wiki/Autovivification)) is quite clear 
in a[b][c]++, so it makes sense to enable the expression.


Andrei


Re: Wrong lowering for a[b][c]++

2012-03-21 Thread Jonathan M Davis
On Wednesday, March 21, 2012 15:52:58 H. S. Teoh wrote:
 On Wed, Mar 21, 2012 at 05:21:22PM -0400, Jonathan M Davis wrote:
  On Wednesday, March 21, 2012 12:00:36 H. S. Teoh wrote:
 [...]
 
   But perhaps that was the wrong example to use. What if you wanted to
   do this:
   
   map[abc][20] = 1;
   
   Currently this doesn't work. You have to explicitly create
   map[abc] first.
  
  Which I think makes perfect sense. I think that implicitly creating
  elements is evil.
 
 [...]
 
 I disagree. If you wrote:
 
 if (map[abc][2] == 1)
 
 then I agree that this should throw a RangeError.
 
 But in the case of writing map[abc][20]=1, you're explicitly saying
 assign 1 to map[abc][20]. This means the user *wants* to create a
 new entry in the AA. So it should create the intermediate entry
 necessary as well.
 
 Otherwise, by your reasoning, this should fail too:
 
 int[string] aa;
 aa[abc] = 1; // should throw RangeError since entry doesn't exist
 
 which I think reduces the usability of the indexing syntax.

Of course that shouldn't throw. You're explicitly creating the element in the 
AA.

 Having the opIndexAssign syntax work only when there's a single level of
 indexing seems to me completely arbitrary.

Not at all. Until you assign the a value to the key in the AA, there's no 
second level to do anything with. To do anything else would be to use the 
value before it's been added to the AA.

- Jonathan M Davis