Bug 16573 – string-typed enum values pass isSomeString but not isInputRange
Status
RESOLVED
Resolution
FIXED
Severity
enhancement
Priority
P1
Component
phobos
Product
D
Version
D2
Platform
x86_64
OS
Linux
Creation time
2016-10-02T11:57:00Z
Last change time
2016-12-20T17:02:28Z
Assigned to
razvan.nitu1305
Creator
ryan
Comments
Comment #0 by ryan — 2016-10-02T11:57:37Z
---
import std.traits, std.range;
enum S { foo = "foo" }
static assert(isSomeString!S); // true
static assert(isInputRange!S); // false
---
I would expect that all strings are input ranges, so this result is confusing. In particular, it caused code to break when https://github.com/dlang/phobos/pull/3447 was merged.
The following used to work:
---
enum S { foo = "foo" }
import std.file;
assert(S.foo.exists);
---
The PR range-ified std.file.exists, which should still allow it to accept all the same types it did before. However, it now fails for S.foo as it is a string but not a range. I think S.foo should either be both a string and a range, or neither.
Related: https://issues.dlang.org/show_bug.cgi?id=8508
Comment #1 by issues.dlang — 2016-10-02T14:37:34Z
The reason that it's not considered an input range is that it doesn't compile with popFront. e.g.
void main()
{
import std.range;
enum S { foo = "foo" }
S s;
s.popFront();
}
results in
q.d(6): Error: template std.range.primitives.popFront cannot deduce function from argument types !()(S), candidates are:
/usr/local/include/d/phobos/std/range/primitives.d(2039): std.range.primitives.popFront(T)(ref T[] a) if (!isNarrowString!(T[]) && !is(T[] == void[]))
/usr/local/include/d/phobos/std/range/primitives.d(2062): std.range.primitives.popFront(C)(ref C[] str) if (isNarrowString!(C[]))
and it looks like that fails, because popFront takes its argument by ref and that parameter is a dynamic array, and since passing an enum to a function that takes something else requires a converison, and ref doesn't work with conversions, the enum can't be used with popFront even though it converts to a string.
Personally, I'm surprised that isSomeString works with an enum. I thought that we made it so that it didn't, since it was an enum and not actually a string, but it looks like it's been that way for at least a few releases, so I could just be misremembering. In general though, treating implicit conversions like that as if they were the base type is mine field of problems with generic code, so I can't say that I'm particularly pleased that isSomeString is true for an enum, even if its base type is string. isSomeString _does_ properly fail with other types of implicit conversions though.
As for isInputRange, I really don't think that it makes sense for it to be true for an enum, because as soon as you pop the front off, it's bound to no longer be one of the members of the enum. Really, the enum needs to be properly converted to a string before doing anything like that to it.
So, I don't know if isSomeString should be changed (I'd like it if it were, but it's probably too late at this point), but I really don't think that isInputRange should be. So, the question is what to do with exists to fix the regression.
Looking at exists, I think that the problem is isConvertibleToString. For some reason, it doesn't work with enums (which really doesn't make sense given its name). If it did, then exists would work, because the overload whose constraint is isConvertibleToString!R would convert the enum to a string to pass to the other overload, and it would work.
So, I think that what should be done is to make isConvertibleToString work with enums - though given that isSomeString erroneously thinks that an enum with the base type string is a string, that could cause problems. Though for some reason, isConvertibleToString isn't actually documented, so there shouldn't be code in wild using it, and we shouldn't break code outside of Phobos if we made the change. Still, the fact that isSomeString thinks that an enum is a string does make things a bit murky. It really shouldn't.
Comment #2 by ryan — 2016-10-02T15:08:20Z
> Personally, I'm surprised that isSomeString works with an enum
Well, you did merge the PR that made it so :)
https://github.com/dlang/phobos/pull/739
I agree that the simplest and most consistent solution is just reverting that, but it seems like a deliberate decision.
Either way it sounds like isConvertibleToString should return true though.
While we're at it, isConvertibleToString should either be documented or made private. What do you think?
Comment #3 by issues.dlang — 2016-10-02T15:23:52Z
(In reply to ryan from comment #2)
> > Personally, I'm surprised that isSomeString works with an enum
>
> Well, you did merge the PR that made it so :)
> https://github.com/dlang/phobos/pull/739
Oh, lovely. Well, we all make stupid decisions from time to time, and this situation is a bit sticky either way, since some code really doesn't care about whether it's dealing with a string on an enum, but in general, if a conevrsion isn't forced to the base type, generic code is going to have problems whenever it's dealing with an implicit conversion rather.
What's so weird about that whole thing though was that I was so sure that at one point, we changed those traits to be true for enums and then decided that it was a horrible idea and changed them back. So, I really don't know what happened.
> I agree that the simplest and most consistent solution is just reverting
> that, but it seems like a deliberate decision.
> Either way it sounds like isConvertibleToString should return true though.
> While we're at it, isConvertibleToString should either be documented or made
> private. What do you think?
I don't know why it's not properly documented. It seems like it should be. I'm inclined to think that we should fix it to be true for enums, but that does get a bit weird when combined with the fact that isSomeString would also be true in that case.
Comment #4 by razvan.nitu1305 — 2016-12-20T09:30:42Z
This should be closed, right?
Comment #5 by ryan — 2016-12-20T17:02:28Z
(In reply to RazvanN from comment #4)
> This should be closed, right?
I don't know when things get transitioned from fixed to closed, but a fix has been merged to master: https://github.com/dlang/phobos/pull/4833