D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 15884 - Assigning char[] to std.json.JSONValue creates array, not string
Summary: Assigning char[] to std.json.JSONValue creates array, not string
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: phobos (show other issues)
Version: D2
Hardware: All Mac OS X
: P1 minor
Assignee: No Owner
URL:
Keywords: pull
Depends on:
Blocks:
 
Reported: 2016-04-06 10:56 UTC by Lionello Lunesu
Modified: 2020-03-21 03:56 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Lionello Lunesu 2016-04-06 10:56:37 UTC
Repro:

void main(){
  import std.json;
  JSONValue v = "asdf".dup;
  assert(v.type == JSON_TYPE.STRING);//fails
  assert(v.toString == `"asdf"`);//also fails
}

Expected: the JSONValue should be created as a string
Actual: the JSONValue is an array and gets encodes as [97,115,100,102]
Comment 1 Lionello Lunesu 2016-04-06 11:15:09 UTC
Ugh, I understand why it's the way it is.

Should we do .idup in JSONValue? Either way there's going to be an allocation, either for the JSONValue[] or for the string.

The repro might seem convoluted, but I ran into it like this: a method returns a char[], in order to give the caller full r/w control, which is what such a "toString" function is supposed to do. Using this return value (which thanks to "auto" is not obvious) in a JSONValue results in a less than ideal JSON.
Comment 2 basile-z 2016-04-09 07:51:41 UTC
(In reply to Lionello Lunesu from comment #1)
> Ugh, I understand why it's the way it is.
> 
> Should we do .idup in JSONValue? Either way there's going to be an
> allocation, either for the JSONValue[] or for the string.
> 
> The repro might seem convoluted, but I ran into it like this: a method
> returns a char[], in order to give the caller full r/w control, which is
> what such a "toString" function is supposed to do. Using this return value
> (which thanks to "auto" is not obvious) in a JSONValue results in a less
> than ideal JSON.

I have tested a fix and calling idup doesn't break current tests, actually the method that's involved to make it accepts char[] is a template and with attribute inference there is no problem. It's always pure nothrow @safe, safe is a new requirement on the master version of std.json.
Comment 3 github-bugzilla 2016-04-11 12:39:55 UTC
Commits pushed to master at https://github.com/D-Programming-Language/phobos

https://github.com/D-Programming-Language/phobos/commit/329f068d7aedd0371b324c2676a4ebf75f3a4a13
fix issue 15884 - Assigning char[] to std.json.JSONValue creates array, not string

https://github.com/D-Programming-Language/phobos/commit/ada161f0c571733aeb280566d6aff0af1e9396ad
Merge pull request #4176 from BBasile/issue-15884

fix issue 15884 - Assigning char[] to std.json.JSONValue creates array, not string
Comment 4 Lionello Lunesu 2016-04-12 01:34:20 UTC
Ah, you're fast, but what about wchar[] dchar[]? I suggest we use std.utf.toUTF8() to handle all the non-string cases (it does a "idup" for char[], by the way.)
Comment 5 Lionello Lunesu 2016-04-12 01:52:00 UTC
I don't know how the link magic works, but here's the PR:
https://github.com/D-Programming-Language/phobos/pull/4185
Comment 6 github-bugzilla 2016-04-17 14:07:12 UTC
Commits pushed to master at https://github.com/dlang/phobos

https://github.com/dlang/phobos/commit/1f1ed031a3215b3fa6585ccca189d68c1ad5b154
Fix issue 15884 Encode wstring/wchar[]/dstring/dchar[] as JSON string

https://github.com/dlang/phobos/commit/1f37557e38b787a021da1cce4c46c4c8ae131d04
Merge pull request #4185 from lionello/fix15884

Fix issue 15884 Encode wstring/wchar[]/dstring/dchar[] as JSON string