D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 16642 - byCodeUnit doesn't work AutodecodableStrings unless they're actually strings or alias a variable that's a string
Summary: byCodeUnit doesn't work AutodecodableStrings unless they're actually strings ...
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: phobos (show other issues)
Version: D2
Hardware: All All
: P1 normal
Assignee: No Owner
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-27 08:09 UTC by Jonathan M Davis
Modified: 2018-01-05 13:27 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 Jonathan M Davis 2016-10-27 08:09:44 UTC
This code fails to compile

import std.utf;

struct S
{
    string s;
    string str() { return s; }
    alias str this;
}

void main()
{
    auto s = S("hello");
    auto range = byCodeUnit(s);
}


Similarly, this fails to compile

import std.utf;

enum E { a = "hello" }

void main()
{
    auto s = E.a;
    auto range = byCodeUnit(s);
}

Also, this compiles but uses the alias rather than the range API and thus fails the assertion:

import std.range.primitives;
import std.utf;

static struct RangeAndStringish
{
    string data;

    string s;
    alias s this;

    bool empty() { return data.empty; }
    char front() { return data[0]; }
    void popFront() { data = data[1 .. $]; }
}

void main()
{
    auto fn = RangeAndStringish("test.d", "other");
    auto x = fn.byCodeUnit();
    assert(x.front == 't');
}

I suppose that it could be argued that this last case is doing the right thing - or at least that it's ambiguous, but normally, an implicit conversion is only supposed to be used if the type itself doesn't work directly, and in this case, the type itself would work without the implicit conversion.
Comment 1 Jonathan M Davis 2016-10-27 09:28:05 UTC
https://github.com/dlang/phobos/pull/4879
Comment 2 github-bugzilla 2017-03-10 21:02:59 UTC
Commits pushed to master at https://github.com/dlang/phobos

https://github.com/dlang/phobos/commit/6e3d41cfe92d0af0360002db6436d2ec98f503af
Fix issue 16642: byCodeUnit doesn't work properly with alias this.

If a template is going to allow implicit conversions, it really needs to
force the conversion in order to avoid subtle bugs. This fixes
byCodeUnit so that it forces the conversion to a string type for types
that convert implicitly. It also fixes it so that types which implicitly
convert to a string type but are also ranges of characters without the
conversion will not be converted by byCodeUnit.

https://github.com/dlang/phobos/commit/13b3da64a81360e96bab3d1bb09a3a42b751da36
Merge pull request #4879 from jmdavis/issue_16642

Fix Issue 16642  - byCodeUnit doesn't work properly with alias this.
Comment 3 github-bugzilla 2017-03-22 12:22:32 UTC
Commits pushed to stable at https://github.com/dlang/phobos

https://github.com/dlang/phobos/commit/6e3d41cfe92d0af0360002db6436d2ec98f503af
Fix issue 16642: byCodeUnit doesn't work properly with alias this.

https://github.com/dlang/phobos/commit/13b3da64a81360e96bab3d1bb09a3a42b751da36
Merge pull request #4879 from jmdavis/issue_16642
Comment 4 github-bugzilla 2017-08-07 12:26:21 UTC
Commits pushed to newCTFE at https://github.com/dlang/phobos

https://github.com/dlang/phobos/commit/6e3d41cfe92d0af0360002db6436d2ec98f503af
Fix issue 16642: byCodeUnit doesn't work properly with alias this.

https://github.com/dlang/phobos/commit/13b3da64a81360e96bab3d1bb09a3a42b751da36
Merge pull request #4879 from jmdavis/issue_16642
Comment 5 github-bugzilla 2018-01-05 13:27:38 UTC
Commits pushed to dmd-cxx at https://github.com/dlang/phobos

https://github.com/dlang/phobos/commit/6e3d41cfe92d0af0360002db6436d2ec98f503af
Fix issue 16642: byCodeUnit doesn't work properly with alias this.

https://github.com/dlang/phobos/commit/13b3da64a81360e96bab3d1bb09a3a42b751da36
Merge pull request #4879 from jmdavis/issue_16642