Re: Wrong lowering for a[b][c]++
Andrej Mitrovic: > On 3/23/12, Timon Gehr 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]++
On 3/23/12, Timon Gehr 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]++
On 03/23/2012 09:10 PM, Andrej Mitrovic wrote: On 3/23/12, Timon Gehr 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]++
On 3/23/12, Timon Gehr 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]++
On 03/23/2012 07:15 AM, Andrej Mitrovic wrote: On 3/23/12, H. S. Teoh 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]++
"H. S. Teoh" 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]++
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]++
On 23 March 2012 19:15, Andrej Mitrovic 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]++
On 3/23/12, H. S. Teoh 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]++
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.
Re: Wrong lowering for a[b][c]++
On 3/23/12, H. S. Teoh 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]++
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 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]++
On Thu, Mar 22, 2012 at 10:25:04PM +0100, Andrej Mitrovic wrote: > On 3/22/12, H. S. Teoh 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]++
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]++
On 3/22/12, H. S. Teoh 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]++
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]++
On 22/03/12 18:43, Andrej Mitrovic wrote: On 3/22/12, H. S. Teoh 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]++
> On 3/22/12, H. S. Teoh 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]++
On 3/22/12, H. S. Teoh 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]++
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]++
On Thu, Mar 22, 2012 at 07:04:09PM +0100, Andrej Mitrovic wrote: > On 3/22/12, Andrej Mitrovic 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]++
On 3/22/12, H. S. Teoh 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]++
On 3/22/12, Andrej Mitrovic 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]++
On Thu, Mar 22, 2012 at 06:43:29PM +0100, Andrej Mitrovic wrote: > On 3/22/12, H. S. Teoh 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]++
On 3/22/12, H. S. Teoh 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]++
On Thu, 22 Mar 2012 15:35:06 +0100, H. S. Teoh wrote: On Thu, Mar 22, 2012 at 03:26:00PM +0100, Andrej Mitrovic wrote: On 3/22/12, Jesse Phillips 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]++
On Thu, Mar 22, 2012 at 05:52:54PM +0100, Andrej Mitrovic wrote: > On 3/22/12, H. S. Teoh 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]++
On 3/22/12, H. S. Teoh 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]++
On Thu, Mar 22, 2012 at 03:48:24PM +0100, Andrej Mitrovic wrote: > On 3/22/12, H. S. Teoh 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]++
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]++
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. 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 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]++
On 3/22/12, H. S. Teoh 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]++
On Thu, Mar 22, 2012 at 03:26:00PM +0100, Andrej Mitrovic wrote: > On 3/22/12, Jesse Phillips 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]++
On Thu, Mar 22, 2012 at 11:01:35AM +0100, Andrej Mitrovic wrote: > On 3/21/12, 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 > > 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]++
On 3/22/12, Jesse Phillips 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]++
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]++
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]++
On 3/21/12, 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 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]++
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
Re: Wrong lowering for a[b][c]++
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]++
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]++
On Wed, Mar 21, 2012 at 09:25:04PM +0100, Andrej Mitrovic wrote: > On 3/21/12, H. S. Teoh 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]++
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]++
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]++
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]++
On 3/21/12, H. S. Teoh 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]++
On Wed, 21 Mar 2012 14:29:14 -0400, 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 ++. 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]++
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]++
On Wed, Mar 21, 2012 at 07:56:47PM +0100, Andrej Mitrovic wrote: > On 3/21/12, H. S. Teoh 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]++
On 3/21/12, Ali Çehreli wrote: > This works: For some reason thought it didn't work without double-checking. Heh. :p
Re: Wrong lowering for a[b][c]++
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]++
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]++
On 3/21/12, H. S. Teoh 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]++
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]++
On 3/21/12, Jonathan M Davis 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]++
On 3/21/12, H. S. Teoh 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]++
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]++
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
Wrong lowering for a[b][c]++
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