Bug 15027 – rangified functions no longer work with alias this'ed strings (e.g. DirEntry)

Status
RESOLVED
Resolution
FIXED
Severity
regression
Priority
P1
Component
phobos
Product
D
Version
D2
Platform
All
OS
All
Creation time
2015-09-08T06:59:00Z
Last change time
2015-10-28T10:31:03Z
Keywords
rejects-valid
Assigned to
nobody
Creator
r.sagitario
See also
https://issues.dlang.org/show_bug.cgi?id=14765

Comments

Comment #0 by r.sagitario — 2015-09-08T06:59:31Z
On git-head, this loop no longer compiles: import std.file; string foo() { foreach(f; dirEntries("/", "*", SpanMode.shallow, false)) if(std.file.isDir(f)) return f; return null; } >dmd -c test.d test.d(8): Error: template std.file.isDir cannot deduce function from argument types !()(DirEntry), candidates are: c:\s\d\rainers\phobos\std\file.d(1399): std.file.isDir(R)(R name) if (isInputRange!R && isSomeChar!(ElementEncodingType!R)) compiling with dmd-2.068.1 is ok. The problem seems to be that the rangified file functions no longer accept arguments that alias this to strings.
Comment #1 by k.hara.pg — 2015-09-11T03:49:58Z
It's hard to resolve issue with current D language features. Reduced case: ---- void popFront(T)(ref T[] a) { a = a[1..$]; } enum bool isInputRange(R) = is(typeof( { R r; r.popFront(); })); struct DirEntry { @property string name() { return ""; } alias name this; } pragma(msg, isInputRange!DirEntry); // prints 'false' pragma(msg, isInputRange!(typeof(DirEntry.init.name))); // prints 'true' bool isDir(R)(R r) if (isInputRange!R) { return true; } void main() { DirEntry de; bool c = isDir(de); // [A] cannot match! } At line [A], `isDir` template function tries to deduce R from the function argument `de`. Although the type DirEntry matches R and deduction succeeds, IFTI fails to satisfy the template constraint isInputRange!R. What we need is R will be deduced to string, which is the type of alias this expression `de.name`. Sadly template constraint evaluation happens *after* the each template parameter deductions succeeds. In other words, currently we have no way to reject the match of R with DirEntry. A workaround code would be something like: bool isDir(Range)(Range range) if (isInputRange!Range || is(StringTypeOf!Range)) { static if (is(StringTypeOf!Range) && isAggregate!Range) alias R = StringTypeOf!Range; else alias R = Range; static bool isDirImpl(R r) { ... } return isDirImpl(range); // expect alias this expansion on function arguments } However, it requires redundant copy and destruction of DirEntry. So it would not become a solution.
Comment #2 by r.sagitario — 2015-09-11T06:09:18Z
The workaround might be a little better if you just make a copy of type StringTypeOf!Range excluding everything but the name string. It doe not work for aggregates that alias this to some other input range, though. The main problem is that DirEntry does not qualify as an InputRange, because the aliased property returns an rvalue. This cannot be passed to popFront through UFCS as it expectes a ref argument. Maybe a more general solution is to check whether an AliasThisTypeOf exists and a saved copy of it can be used as an input range.
Comment #3 by bugzilla — 2015-10-03T00:23:42Z
(In reply to Rainer Schuetze from comment #2) > The main problem is that DirEntry does not qualify as an InputRange, because > the aliased property returns an rvalue. This cannot be passed to popFront > through UFCS as it expectes a ref argument. I think this is correct. If you remove the 'ref' from 'popFront', it works. It also works if DirEntry is defined as: string name; alias name this; i.e. then 'name' becomes an lvalue. popFront() cannot update an rvalue.
Comment #4 by bugzilla — 2015-10-03T01:56:55Z
The trouble is InputRange has to modify the range. A solution is to rewrite DirEntry.name from: @property string name() const pure nothrow { return _name; } to: string _name2; @property ref string name() return const pure nothrow { _name2 = _name; return _name2; } but there'll be trouble with that if there are two calls to name() within the same expression and both try to modify through the ref.
Comment #5 by bugzilla — 2015-10-05T03:06:14Z
I thought about this for quite a while. The bottom line is DirEntry is being too clever in using alias this to wrap a string, and should simply stick with DirEntry.name when the name is desired. Rationale: An InputRange, by its very nature, consumes its input. If DirEntry were made into an InputRange, then it would consume the name and eventually the name string will be empty. DirEntry's alias this is returning a copy of its data, meaning that even if it compiled as an InputRange, the popFront would never advance, as it would keep getting reset every time. So, the solution can only be one of: 1. DirEntry is an InputRange. 2. DirEntry is converted to an InputRange before passing to isDir(). The trouble with (1) is that DirEntry then becomes consumed, which would be surprising behavior. (2) can be accomplished by not using alias this, but just using de.name() directly. Or, (2) can be accomplished by overloading isDir() to accept string arguments. But this implies adding an overload to every function that accepts an InputRange. This is out of the question. Leaving us with rewriting isDir(de) to isDir(de.name). That seems to be the most practical solution.
Comment #6 by r.sagitario — 2015-10-05T06:26:10Z
> Leaving us with rewriting isDir(de) to isDir(de.name). > That seems to be the most practical solution. While feasible in the case of dirEntry (though breaking existing code), it will not help in the more general case, i.e. you cannot call a template function if the alias this type matches the constraints, but the struct type does not. Maybe template argument type deduction should just try the aliased type if the struct type itself fails. Doesn't this fit with other rules regarding "alias this"?
Comment #7 by bugzilla — 2015-10-06T04:02:41Z
(In reply to Rainer Schuetze from comment #6) > Maybe template argument type deduction should just try the aliased type if > the struct type itself fails. Doesn't this fit with other rules regarding > "alias this"? The trouble is that it fails in the constraint, not the type deduction part.
Comment #8 by r.sagitario — 2015-10-06T06:44:25Z
> The trouble is that it fails in the constraint, not the type deduction part. It still fails, so instead of emitting an error message the compiler could change the type of the function arguments to the aliased type and retry deduction and constraint check. For multiple arguments with alias this, this could lead to a large number of possible combinations to try, though.
Comment #9 by code — 2015-10-06T06:59:15Z
*** Issue 15057 has been marked as a duplicate of this issue. ***
Comment #10 by code — 2015-10-06T07:05:24Z
(In reply to Walter Bright from comment #5) > Or, (2) can be accomplished by overloading isDir() to accept string > arguments. But this implies adding an overload to every function that > accepts an InputRange. This is out of the question. What's the problem with that? It's rather good to precompile the common template instantiations into phobos.
Comment #11 by code — 2015-10-06T07:08:44Z
Slight variation of the bug where the aliased string is an lvalue, but cannot be assigned. cat > bug.d << CODE struct InternedString { void opAssign(InternedString other) { this.data = other.data; } string data; alias data this; } auto bug(InternedString s) { import std.file : exists; return exists(s); } CODE This breaks b/c some code in utf.d tries to `r = r[1 .. $]` slice an InternedString.
Comment #12 by bugzilla — 2015-10-07T04:08:53Z
(In reply to Martin Nowak from comment #10) > (In reply to Walter Bright from comment #5) > > Or, (2) can be accomplished by overloading isDir() to accept string > > arguments. But this implies adding an overload to every function that > > accepts an InputRange. This is out of the question. > > What's the problem with that? It's rather good to precompile the common > template instantiations into phobos. The problem is that it pretty much doubles the number of functions in Phobos. And means that everyone who writes a reusable library has to have double functions.
Comment #13 by bugzilla — 2015-10-07T04:14:31Z
(In reply to Rainer Schuetze from comment #8) > It still fails, so instead of emitting an error message the compiler could > change the type of the function arguments to the aliased type and retry > deduction and constraint check. For multiple arguments with alias this, this > could lead to a large number of possible combinations to try, though. Given how overloading even now can lead to inexplicable seeming results, I view such an additional layer of complexity as a looming disaster, especially with the combinatorial aspect of it. I have thought about something like: struct foo(T : isInputRange) { ... } where the constraint would become part of the type deduction logic, but that's a major addition to the language, not something we can just throw in.
Comment #14 by bugzilla — 2015-10-07T07:09:53Z
Comment #15 by r.sagitario — 2015-10-07T19:59:09Z
>Check this out: >https://github.com/D-Programming-Language/phobos/pull/3694 This is pretty much the workaround given by Kenji above. It has the downside that you have to copy the full DirEntry object to the stack as a function argument. Not a big issue for DirEntry, but it can be pretty bad if the struct has an expensive postblit or if it is disabled.
Comment #16 by r.sagitario — 2015-10-09T06:48:52Z
Here is a version that overloads structs with alias this to a string range by reference, avoiding the copy problem: auto isDir(Range)(Range input) if (isForwardRange!Range && isSomeChar!(ElementEncodingType!Range)) { //... } auto isDir(Range)(ref Range input) if (!(isForwardRange!Range && isSomeChar!(ElementEncodingType!Range)) && is(StringTypeOf!Range)) { return isDir(cast(StringTypeOf!Range) input); }
Comment #17 by r.sagitario — 2015-10-09T06:59:35Z
>auto isDir(Range)(ref Range input) "auto ref" seems to work even better.
Comment #18 by code — 2015-10-14T13:36:34Z
Lots of small PR fixups already, but we need to consistently check/fix all of the rangified functions. D-Programming-Language/phobos: Pull Request #3676 D-Programming-Language/phobos: Pull Request #3702 D-Programming-Language/phobos: Pull Request #3703 D-Programming-Language/phobos: Pull Request #3704 See the changelog for a list. http://dlang.org/changelog/2.068.0.html#rangified-functions http://dlang.org/changelog/2.069.0.html#more-rangified-functions https://trello.com/c/W6JX23lD/113-issue-15027-rangified-functions-no-longer-work-with-alias-this-ed-strings-e-g-direntry
Comment #19 by jakobovrum — 2015-10-23T04:57:18Z
(In reply to Walter Bright from comment #5) > So, the solution can only be one of: > 1. DirEntry is an InputRange. > 2. DirEntry is converted to an InputRange before passing to isDir(). > > The trouble with (1) is that DirEntry then becomes consumed, which would be > surprising behavior. When the range algorithm receives the range by value, then the DirEntry and its member slice are copied and only this copy is consumed, so I'm not sure it would be a big problem. It is essentially the same situation we have with other range types. However, from an encapsulation standpoint, DirEntry.name shouldn't be externally modifiable even for a mutable DirEntry instance. At least I think this is the conservative OOP angle. One intrusive change could be to consider DirEntry a "container" of characters, which presents a range interface with dirEntry[]. It's basically a generalisation of what is going on now with the StringTypeOf changes. I agree that this approach doesn't scale. It puts the onus of converting to range type on the wrong side of the function call. The meat of the algorithm wants a *range of characters*, and the ritual conversion to this range of characters is just a triviality that has nothing to do with the algorithm. What previously happened with passing DirEntry to string functions was that the argument was converted to the character range supertype implicitly *at the call-site*. So maybe simply what the right solution is, even if it's hard, is to redesign IFTI to consider AliasThis.
Comment #20 by github-bugzilla — 2015-10-26T13:57:02Z
Commits pushed to stable at https://github.com/D-Programming-Language/phobos https://github.com/D-Programming-Language/phobos/commit/3a8a9ea7939cf2e5c4a6452597681975ad3fb866 std.file fixes for string-like types - fixes Issue 15027 https://github.com/D-Programming-Language/phobos/commit/384410aa188f4300b80d7f6dbf2145ecc8c03e92 Merge pull request #3770 from MartinNowak/string_like fix Issue 15027 – rangified functions no longer work with string-like types
Comment #21 by github-bugzilla — 2015-10-28T10:31:03Z