Bug 14682 – [REG2.037] Incorrect interpretation of ~ []

Status
RESOLVED
Resolution
FIXED
Severity
regression
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2015-06-11T04:56:00Z
Last change time
2017-07-22T12:35:28Z
Keywords
pull, wrong-code
Assigned to
nobody
Creator
dlang-bugzilla

Comments

Comment #0 by dlang-bugzilla — 2015-06-11T04:56:40Z
////////// test.d ////////// import std.stdio; void main() { auto foo = ["foo"] ~ []; assert(foo.length == 1); } //////////////////////////// For some reason, DMD interprets `~[]` as `~[""]`.
Comment #1 by dlang-bugzilla — 2015-06-11T04:57:39Z
> import std.stdio; Sorry, that's not actually needed.
Comment #2 by dlang-bugzilla — 2015-06-11T05:17:10Z
Comment #3 by k.hara.pg — 2015-06-11T06:34:24Z
> For some reason, DMD interprets `~[]` as `~[""]`. Because: auto foo = ["foo"] ~ []; is equivalent to: auto foo = ["foo"] ~ ""; At a concatenation expression, arr ~ elem is preferred than arr ~ arr2 when elem is implicitly convertible to typeof(arr[i]). In the code, the rhs `[]` is implicitly convertible to typeof("foo") == string.
Comment #4 by dlang-bugzilla — 2015-06-11T06:40:01Z
Thanks for the clarification. Do you think it should be fixed? Any way you turn it, the behavior might be unexpected: auto foo = ["foo"] ~ ['x']; -- or -- auto foo = ["foo"] ~ ["x"]; Remove the only element in the array, and something surprising may happen, depending on what you expect.
Comment #5 by dlang-bugzilla — 2015-06-11T09:26:34Z
I think it should be fixed, because if you want to append [""], you can do so explicitly, but if you want to append an empty array, there is no convenient syntax for it. I ran into it with the following code: run([make, "-f", makeFileName, "all", "kindle", "pdf", "verbatim", "LATEST=" ~ latest, ] ~ (config.noDateTime ? ["NODATETIME=nodatetime.ddoc"] : []) ~ [ ], sourceDir); Here I wanted to make an array element conditionally present, so I used: ] ~ (cond ? [elem] : []) ~ [ inside an array literal. The problem is that this line's meaning changes on whether it is the last line in the array literal or not.
Comment #6 by schveiguy — 2015-06-11T14:12:46Z
Could you do: (config.noDateTime ? ["NODATETIME=nodatetime.ddoc"] : string[].init) Ugly I know :) I agree the usage is surprising. And actually, it's not the same as appending "", because "" is not null, whereas [] is.
Comment #7 by k.hara.pg — 2015-06-11T14:39:29Z
(In reply to Vladimir Panteleev from comment #5) > I think it should be fixed, because if you want to append [""], you can do > so explicitly, but if you want to append an empty array, there is no > convenient syntax for it. What you want is: define `[]` as a representation of "identity element" (http://en.wikipedia.org/wiki/Identity_element) for arbitrary concat operation? Hmm, probably I can agree it's necessary thing.
Comment #8 by dlang-bugzilla — 2015-06-11T21:10:45Z
(In reply to Steven Schveighoffer from comment #6) > Could you do: > > (config.noDateTime ? ["NODATETIME=nodatetime.ddoc"] : string[].init) That's not where the problem lies - this was just an example.
Comment #9 by schveiguy — 2015-06-11T23:43:07Z
(In reply to Vladimir Panteleev from comment #8) > (In reply to Steven Schveighoffer from comment #6) > > Could you do: > > > > (config.noDateTime ? ["NODATETIME=nodatetime.ddoc"] : string[].init) > > That's not where the problem lies - this was just an example. I was responding to this: (In reply to Vladimir Panteleev from comment #5) > I think it should be fixed, because if you want to append [""], you can do > so explicitly, but if you want to append an empty array, there is no > convenient syntax for it. I suppose you can call that "inconvenient" syntax, but it's not that bad IMO.
Comment #10 by dlang-bugzilla — 2015-06-11T23:47:00Z
(In reply to Steven Schveighoffer from comment #9) > (In reply to Vladimir Panteleev from comment #8) > > (In reply to Steven Schveighoffer from comment #6) > > > Could you do: > > > > > > (config.noDateTime ? ["NODATETIME=nodatetime.ddoc"] : string[].init) > > > > That's not where the problem lies - this was just an example. > > I was responding to this: Yes. This is not where the problem lies. The problem is not with the ternary statement, but with the ~[] after it. And yes, this is one possible but inconvenient syntax to replace [].
Comment #11 by schveiguy — 2015-06-12T00:27:30Z
(In reply to Vladimir Panteleev from comment #10) > Yes. This is not where the problem lies. The problem is not with the ternary > statement, but with the ~[] after it. And yes, this is one possible but > inconvenient syntax to replace []. OOOHHHH! oops :) Hm... looking at that, well, I don't see a huge problem, just remove the trailing []. But then again, you could be doing something generative. In either case, I agree that the the syntax is ambiguous. I don't know what should be done, quite possibly the only viable thing is to error on [], and make you specify the type you are trying to do. Otherwise, you'd silently change the meaning of the code. That sucks. I would expect probably the answer is to leave it the way it is, and require the workaround. For strings, it's pretty annoying, because nobody ever creates string literals via [], but for something like int[][], it's not so cut and dry.
Comment #12 by k.hara.pg — 2015-06-12T09:29:14Z
(In reply to Kenji Hara from comment #3) > At a concatenation expression, arr ~ elem is preferred than arr ~ arr2 when > elem is implicitly convertible to typeof(arr[i]). In the code, the rhs `[]` > is implicitly convertible to typeof("foo") == string. Sorry I was mistaken. Normally D's array concatenation prefers two arrays concat, rather than prepending/appending one element. I noticed a comment in CatExp::semantic explains how the ambiguity should be resolved to. if (tb1next && tb2next && (tb1next->implicitConvTo(tb2next) >= MATCHconst || tb2next->implicitConvTo(tb1next) >= MATCHconst ) ) { /* Here to avoid the case of: * void*[] a = [cast(void*)1]; * void*[] b = [cast(void*)2]; * a ~ b; * becoming: * a ~ [cast(void*)b]; */ } And in CatAssignExp::semantic, array appending is already preferred than element appending. int[] a1; int[][] a2; int[][][] a3; import std.stdio; { auto x = a1 ~ []; x.writeln(); } // prints [] { auto x = a2 ~ []; x.writeln(); } // prints [] { auto x = a3 ~ []; x.writeln(); } // prints [] Therefore I can say it's just an implementation bug in dmd.
Comment #13 by k.hara.pg — 2015-06-13T05:47:21Z
Comment #14 by github-bugzilla — 2015-06-17T18:50:20Z
Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/4d5c653d83984e85ba3976702c4f59a95b8d087a fix Issue 14682 - Incorrect interpretation of ~ [] CatExp should check whether array literal operands can fit to opposite side type. https://github.com/D-Programming-Language/dmd/commit/67b82717d2e189974a1662ded8a8229791977eb9 Merge pull request #4742 from 9rnsr/fix14682 [REG2.037] Issue 14682 - Incorrect interpretation of ~ []
Comment #15 by github-bugzilla — 2015-06-26T14:11:49Z
Commit pushed to stable at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/e23ff7c0e4f9de3f4dd6352e261ee430466d7ddb Merge pull request #4742 from 9rnsr/fix14682 [REG2.037] Issue 14682 - Incorrect interpretation of ~ []
Comment #16 by github-bugzilla — 2015-07-24T03:20:07Z
Comment #17 by github-bugzilla — 2015-10-04T18:18:30Z
Comment #18 by github-bugzilla — 2017-07-22T12:35:28Z
Commits pushed to dmd-cxx at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/4d5c653d83984e85ba3976702c4f59a95b8d087a fix Issue 14682 - Incorrect interpretation of ~ [] https://github.com/dlang/dmd/commit/67b82717d2e189974a1662ded8a8229791977eb9 Merge pull request #4742 from 9rnsr/fix14682