Bug 5233 – [patch] std.range.put accepts *any* element type when putting to an array.

Status
RESOLVED
Resolution
FIXED
Severity
normal
Priority
P2
Component
phobos
Product
D
Version
D2
Platform
Other
OS
Windows
Creation time
2010-11-18T00:13:00Z
Last change time
2014-10-10T20:32:57Z
Keywords
patch, pull
Assigned to
andrei
Creator
sandford
Blocks
5813

Comments

Comment #0 by sandford — 2010-11-18T00:13:49Z
Essentially, there is a lack of a static assert in some of the branches of the put function, leading to any element being accepted for certain input ranges (notably strings) which define a non-assignable 'front'. Unfortunately, ding template constraints result in a recursive expansion with isOutputRange. I've listed a patch below which adds the static asserts and special cases strings so they work as expected. Oh, here's a simple unit test: assert( !isOutputRange!(char[],real) ); Patch: void put(R, E)(ref R r, E e) { // BUG, adding template constraints result in a recursive expansion with isOutputRange static if ( __traits(compiles, {return R.init.put(E.init);}) ) { // Patch from Issue 4880 // commit to using the "put" method static if (!isArray!R && is(typeof(r.put(e)))) { r.put(e); } else static if (!isArray!R && is(typeof(r.put((&e)[0..1])))) { r.put((&e)[0..1]); } else { static assert(false, "Cannot put a "~E.stringof~" into a "~R.stringof); } } else { static if (isInputRange!R) { // Commit to using assignment to front static if (is(typeof(r.front = e, r.popFront()))) { r.front = e; r.popFront(); } else static if (isInputRange!E && is(typeof(put(r, e.front)))) { for (; !e.empty; e.popFront()) put(r, e.front); } else static if( isSomeString!R && isSomeChar!E ) { static if( (typeof(r[0])).sizeof < E.sizeof ) { // must do some transcoding around here to support char[].put(dchar) Unqual!(typeof(r[0]))[(typeof(r[0])).sizeof == 1 ? 4 : 2] encoded; auto len = std.utf.encode(encoded, e); writeln( typeof(encoded).stringof,"\t", typeof(r[0]).sizeof ,"\t", E.sizeof ); foreach(c;encoded[0 .. len]) { r[0] = c; r.popFront; } } else { r[0] = e; r.popFront; } } else { static assert(false, "Cannot put a "~E.stringof~" into a "~R.stringof); } } else { // Commit to using opCall static if (is(typeof(r(e)))) { r(e); } else static if (is(typeof(r((&e)[0..1])))) { r((&e)[0..1]); } else { static assert(false, "Cannot put a "~E.stringof~" into a "~R.stringof); } } } }
Comment #1 by sandford — 2010-11-18T12:02:54Z
An updated patch allowing char[]/wchar[] to be put into ubyte/ushort output ranges. void put(R, E)(ref R r, E e) { // BUG, adding template constraints result in a recursive expansion with isOutputRange static if ( __traits(compiles, {return R.init.put(E.init);}) ) { // Patch from Issue 4880 // commit to using the "put" method static if (!isArray!R && is(typeof(r.put(e)))) { r.put(e); } else static if (!isArray!R && is(typeof(r.put((&e)[0..1])))) { r.put((&e)[0..1]); } else { static assert(false, "Cannot put a "~E.stringof~" into a "~R.stringof); } } else { static if (isInputRange!R) { // Commit to using assignment to front static if (is(typeof(r.front = e, r.popFront()))) { r.front = e; r.popFront(); } else static if (isInputRange!E && is(typeof(put(r, e.front)))) { pragma(msg,R,"\t",E," ====="); for (; !e.empty; e.popFront()) put(r, e.front); } else static if( isSomeString!R && isSomeChar!E ) { static if( (typeof(r[0])).sizeof < E.sizeof ) { // must do some transcoding around here to support char[].put(dchar) Unqual!(typeof(r[0]))[(typeof(r[0])).sizeof == 1 ? 4 : 2] encoded; auto len = std.utf.encode(encoded, e); writeln( typeof(encoded).stringof,"\t", typeof(r[0]).sizeof ,"\t", E.sizeof ); foreach(c;encoded[0 .. len]) { r[0] = c; r.popFront; } } else { r[0] = e; r.popFront; } } else static if(hasSlicing!E && is(typeof(e.length == size_t)) && is(typeof(put(r, e[0]))) ) { for (; !e.empty; e = e[1..e.length]) put(r, e[0]); } else { static assert(false, "Cannot put a "~E.stringof~" into a "~R.stringof); } } else { // Commit to using opCall static if (is(typeof(r(e)))) { r(e); } else static if (is(typeof(r((&e)[0..1])))) { r((&e)[0..1]); } else { static assert(false, "Cannot put a "~E.stringof~" into a "~R.stringof); } } } }
Comment #2 by sandford — 2010-11-18T13:23:19Z
*sigh* Patch version 3. Added support for putting an input range into an output range. Unit test: assert( isOutputRange(Appender!(ubyte[]),string) ); Patch: void put(R, E)(ref R r, E e) { // BUG, adding template constraints result in a recursive expansion with isOutputRange static if ( __traits(compiles, {return R.init.put(E.init);}) ) { // Patch from Issue 4880 // commit to using the "put" method static if (!isArray!R && is(typeof(r.put(e)))) { r.put(e); } else static if (!isArray!R && is(typeof(r.put((&e)[0..1])))) { r.put((&e)[0..1]); } else { static assert(false, "Cannot put a "~E.stringof~" into a "~R.stringof); } } else { static if (isInputRange!R) { // Commit to using assignment to front static if (is(typeof(r.front = e, r.popFront()))) { r.front = e; r.popFront(); } else static if (isInputRange!E && is(typeof(put(r, e.front)))) { for (; !e.empty; e.popFront()) put(r, e.front); } else static if( isSomeString!R && isSomeChar!E ) { static if( (typeof(r[0])).sizeof < E.sizeof ) { // must do some transcoding around here to support char[].put(dchar) Unqual!(typeof(r[0]))[(typeof(r[0])).sizeof == 1 ? 4 : 2] encoded; auto len = std.utf.encode(encoded, e); writeln( typeof(encoded).stringof,"\t", typeof(r[0]).sizeof ,"\t", E.sizeof ); foreach(c;encoded[0 .. len]) { r[0] = c; r.popFront; } } else { r[0] = e; r.popFront; } } else static if(hasSlicing!E && is(typeof(e.length == size_t)) && is(typeof(put(r, e[0]))) ) { for (; !e.empty; e = e[1..e.length]) put(r, e[0]); } else { static assert(false, "Cannot put a "~E.stringof~" into a "~R.stringof); } } else { // Commit to using opCall static if (is(typeof(r(e)))) { r(e); } else static if (is(typeof(r((&e)[0..1])))) { r((&e)[0..1]); } else static if (isInputRange!E && is(typeof(put(r, e.front)))) { for (; !e.empty; e.popFront()) put(r, e.front); } else static if(hasSlicing!E && is(typeof(e.length == size_t)) && is(typeof(put(r, e[0]))) ) { for (; !e.empty; e = e[1..e.length]) put(r, e[0]); } else { static assert(false, "Cannot put a "~E.stringof~" into a "~R.stringof); } } } }
Comment #3 by sandford — 2011-03-15T22:22:43Z
Patch version 4. (DMD 2.052, Line 272+) * Removed some debug code that was accidentally included in the last version * Refactored and reformatted * Fixed/added support for using character ranges as output ranges * Added an associated unittest * Requires the removal of unittest two static asserts: static assert(!__traits(compiles, put(a, 'a'))); static assert(!__traits(compiles, put(a, "ABC"))); * put's other unit tests have been tested using DMD 2.052 // BUG template constraints result in a recursive expansion with isOutputRange void put(R, E)(ref R r, E e) { // Patch from Issue 4880 static if ( __traits(compiles, {return R.init.put(E.init);}) ) { // commit to using the "put" method static if (is(typeof(r.put(e)))) r.put(e); else static if (is(typeof(r.put((&e)[0..1])))) r.put((&e)[0..1]); else static assert(false, "Cannot put a "~E.stringof~" into a "~R.stringof); } else static if( isSomeString!R && isSomeChar!E ) { static if( (typeof(r[0])).sizeof < E.sizeof ) { // Transcoding is required to support char[].put(dchar) Unqual!(typeof(r[0]))[(typeof(r[0])).sizeof == 1 ? 4 : 2] encoded; auto len = std.utf.encode(encoded, e); assert(r.length >= len); foreach(c;encoded[0 .. len]) { r[0] = c; r = r[1..$]; } } else { assert(!r.empty); if(r.length == 0) r.length = 1; r[0] = e; r = r[1..$]; } } else static if (isInputRange!R && is(typeof(r.front = e) )) { r.front = e; r.popFront(); } else static if (isInputRange!E && is(typeof(put(r, e.front)))) { for (; !e.empty; e.popFront() ) put(r, e.front); } else static if(hasSlicing!E && hasLength!E && is(typeof(put(r, e[0])))){ for (; e.length > 0; e = e[1..e.length]) put(r, e[0]); } else static if (is(typeof(r(e)))) { // Commit using opCall r(e); } else static if (is(typeof(r((&e)[0..1])))) { r((&e)[0..1]); } else { static assert(false, "Can't put a "~E.stringof~" into a "~R.stringof); } } // char[] as an output range unittest { auto a = "\u2666"d; auto b = "\u2666"; auto s = new char[b.length]; auto t = s; put(t,a); assert( s == b ); }
Comment #4 by sandford — 2011-04-01T22:01:14Z
Patch version 5. *** API Addition *** Optional template boolean parameter asArray. If true, put bypasses testing r.put(e), r.put(e[]), r(e) and r(e[]). This allows templated user defined put routines to accept all valid put types without duplicating all of put's transforms/code snippets. For example: struct Stack(T) { put(U item) if( isOutputRange!(T[],U) ) { static if( is(U:T) ) { // put one item onto the stack } else { .put!(typeof(this),U,true)(this,item); } } } * Updated documentation to reflect changes * Removal of all static asserts. static asserts do not currently compose well with is(typeof(...)) / __traits(compiles,...) statements. Instead, the error message is left as a single line statement. This causes a normal compilation error with a reasonable error message, since the error info is embedded in the bad string, the value of which is included in the error message. * All string put string and string put char cases are UTF correct. * The r(e) snippet now filters out types with ctors but not opCall. * Support was added for index only and foreach-able E types. * Simplification of snippets and code layout [Patch] /** Outputs $(D e) to $(D r). The exact effect is dependent upon the two types. Several cases are accepted, as described below. The code snippets are attempted in order, and the first to compile "wins" and gets evaluated. Setting asArray to true will skip code snippets using user defined $(D put) and opCall methods, allowing templated user defined $(D put) methods to reuse some of $(D put)'s code snippets. $(BOOKTABLE , $(TR $(TH Code Snippet) $(TH Scenario)) $(TR $(TD $(D isSomeString!R && isSomeString!E )) $(TD $(D R) is a mutable string and $(D E) is a string.)) $(TR $(TD $(D isSomeString!R && isSomeChar!E )) $(TD $(D R) is a mutable string and $(D E) is a character.)) $(TR $(TD $(D r.put(e);)) $(TD $(D R) defines a method $(D put) accepting an $(D E).)) $(TR $(TD $(D r.put([ e ]);)) $(TD $(D R) defines a method $(D put) accepting an $(D E[]).)) $(TR $(TD $(D r.front = e; r.popFront();)) $(TD $(D R) is an input range and $(D e) is assignable to $(D r.front).)) $(TR $(TD $(D foreach(v; e) put(r,v))) $(TD Copying range $(D E) to range $(D R).)) $(TR $(TD $(D foreach(i;0..e.length) put(r,e[i]))) $(TD Copying range $(D E) to range $(D R).)) $(TR $(TD $(D r(e);)) $(TD $(D R) is e.g. a delegate accepting an $(D E).)) $(TR $(TD $(D r([ e ]);)) $(TD $(D R) is e.g. a $(D delegate) accepting an $(D E[]).)) ) */ void put(R, E, bool asArray = isArray!R )(ref R r, E e) { static if(isSomeString!R && isSomeString!E && is(typeof(r[0]=r[0])) ){ static if( (typeof(r[0])).sizeof != E.sizeof ) { foreach( typeof(r[0]) v; e) put(r,v); } else { assert(e.length <= r.length); r[0..e.length] = e[]; r = r[e.length..$]; } } else static if(isSomeString!R && isSomeChar!E && is(typeof(r[0]=r[0])) ){ static if( (typeof(r[0])).sizeof < E.sizeof ) { // Transcoding is required to support char[].put(dchar) typeof(r[0])[(typeof(r[0])).sizeof == 1 ? 4 : 2] encoded; auto len = std.utf.encode(encoded, e); put(r,encoded[0 .. len]); } else { assert(!r.empty); r[0] = e; r = r[1..$]; } } else static if( !asArray && __traits(compiles, r.put( e )) ){ r.put( e ); } else static if( !asArray && __traits(compiles, r.put((&e)[0..1] )) ){ r.put((&e)[0..1] ); } else static if (isInputRange!R && is(typeof(r.front = e) )) { r.front = e; r.popFront(); } else static if(__traits(compiles,{foreach(v; e) put(r, v );})){ foreach(v; e) put(r, v ); } else static if(__traits(compiles,{foreach(i;0..e.length) put(r,e[i]);})){ foreach(i;0..e.length) put(r,e[i]); } else static if( _putCall!(R,E,asArray) && is(typeof( r( e )) )){ r( e ); } else static if( _putCall!(R,E,asArray) && is(typeof( r( (&e)[0..1] )) )){ r( (&e)[0..1] ); } else { // @@@BUG@@@ Static asserts can't be combined with is(typeof(put(r,e))) "Can't put a "~E.stringof~" into a "~R.stringof; } } // Helper template for put(): filters out opCall from ctors private template _putCall(R, E, bool asArray = false) { static if(asArray) enum _putCall = false; else static if( is(R==class) || is(R==struct) || is(R==union) ) enum _putCall = is(typeof( (R r, E e){r.opCall( e );}))|| is(typeof( (R r, E e){r.opCall( (&e)[0..1] );})); else enum _putCall = is(typeof( (R r, E e){r( e );}))|| is(typeof( (R r, E e){r( (&e)[0..1] );})); }
Comment #5 by sandford — 2011-04-02T22:22:59Z
Patch version 6. *sigh* Naturally, new ideas come immediately after my previous post. *** API Addition *** ER or R's ElementEncodingType allows user-defined-types to specify a preferred encoding type. The principal point is to prevent the duplication of dchar->char[] conversions in every single output range. i.e. My example from patch 5 didn't handle strings and chars in a UTF correct manner. Now, by setting ER to T, it properly supports chars and strings. struct Stack(T) { put(U item) if( isOutputRange!(T[],U) ) { static if( is(U:T) ) { // put one item onto the stack } else { .put!(typeof(this),U,true,T)(this,item); } } } * Simplification of string handling, which necessitated 3 new code snippets: * The index assign and slice advance * The slice assign and slice advance * The foreach(ER v) put(r,v); * Additional of the r ~= e code snippet. I found I needed to protect this with some additional constraints to prevent the char<->ubyte implicit conversion. Also, this allows put-ing to arrays with const elements. Personally, I think this snippet should take precedence over the input-range snippets, but that would be a major change in behavior. Currently, auto str = "Hello ".dup; auto str2 = str; // Note the need for second of the array put(str2,"World"); assert(str == "World "); Where as with ~= auto str3 = "Hello "; put(str3,"World"); assert(str3 == "Hello World"); I think that viewing arrays as containers first and input-ranges second is more intuitive and useful. [Patch] /** Outputs $(D e) to $(D r). The exact effect is dependent upon the two types. Several cases are accepted, as described below. The code snippets are attempted in order, and the first to compile "wins" and gets evaluated. Setting $(D asArray) to true will skip code snippets using user defined $(D put) and $(D opCall) methods, allowing templated user defined $(D put) methods to reuse some of $(D put)'s code snippets. Furthermore, $(D ER) allows the caller to set a prefered conversion type, enabling put to perform the correct UTF conversions for character and string inputs. $(BOOKTABLE , $(TR $(TH Code Snippet) $(TH Scenario)) $(TR $(TD $(D r.put(e);)) $(TD $(D R) defines a method $(D put) accepting an $(D E).)) $(TR $(TD $(D r.put([ e ]);)) $(TD $(D R) defines a method $(D put) accepting an $(D E[]).)) $(TR $(TD $(D r.front = e; r.popFront();)) $(TD $(D R) is an input range and $(D e) is assignable to $(D r.front).)) $(TR $(TD $(D r[0] = e; r = r[1..$];)) $(TD $(D R) supports slicing and $(D e) is index assignable to $(D r).)) $(TR $(TD $(D r[0..e.length] = e[0..$]; r = r[e.length..$];)) $(TD $(D R) supports slicing and $(D e) is slice assignable to $(D r).)) $(TR $(TD $(D foreach(ER v; e) put(r,v))) $(TD String safe copying of a range $(D E) to range $(D R).)) $(TR $(TD $(D foreach(v; e) put(r,v))) $(TD General copying of a range $(D E) to range $(D R).)) $(TR $(TD $(D foreach(i;0..e.length) put(r,e[i]))) $(TD Copying range $(D E) to range $(D R).)) $(TR $(TD $(D r ~= e;)) $(TD $(D R) supports concatenation with $(D e).)) $(TR $(TD $(D r(e);)) $(TD $(D R) is e.g. a delegate accepting an $(D E).)) $(TR $(TD $(D r([ e ]);)) $(TD $(D R) is e.g. a $(D delegate) accepting an $(D E[]).)) ) */ void put(R, E, bool asArray=isArray!R, ER=ElementEncodingType!R)(ref R r, E e){ static if(isSomeChar!ER && isSomeChar!E && ER.sizeof < E.sizeof ) { // Transcoding is required to support r.put(dchar) ER[ER.sizeof == 1 ? 4 : 2] encoded; auto len = std.utf.encode(encoded, e); put(r,encoded[0 .. len]); } else static if( !asArray && __traits(compiles, r.put( e )) ){ r.put( e ); } else static if( !asArray && __traits(compiles, r.put((&e)[0..1] )) ){ r.put((&e)[0..1] ); } else static if (isInputRange!R && is(typeof(r.front = e) )) { r.front = e; r.popFront(); } else static if(__traits(compiles, {r[0] = e; r = r[1..r.length];} )){ r[0] = e; r = r[1..r.length]; } else static if(__traits(compiles, {r[0..e.length] = e[0..e.length]; r = r[1..r.length]; })){ r[0..e.length] = e[0..e.length]; r = r[e.length..r.length]; } else static if(__traits(compiles,{foreach(ER v; e) put(r, v );})){ foreach(ER v; e) put(r, v ); } else static if(__traits(compiles,{foreach( v; e) put(r, v );})){ foreach( v; e) put(r, v ); } else static if(__traits(compiles,{foreach(i;0..e.length) put(r,e[i]);})){ foreach(i;0..e.length) put(r,e[i]); } else static if(__traits(compiles,r~=e) && (is(ER==void)|| !(isSomeChar!ER^isSomeChar!E))){ r ~= e; } else static if( _putCall!(R,E,asArray) && is(typeof( r( e )) )){ r( e ); } else static if( _putCall!(R,E,asArray) && is(typeof( r( (&e)[0..1] )) )){ r( (&e)[0..1] ); } else { // @@@BUG@@@ Static asserts can't be combined with is(typeof(put(r,e))) "Can't put a "~E.stringof~" into a "~R.stringof ; } } // Helper template for put(): filters out opCall from ctors private template _putCall(R, E, bool asArray = false) { static if(asArray) enum _putCall = false; else static if( is(R==class) || is(R==struct) || is(R==union) ) enum _putCall = is(typeof( (R r, E e){r.opCall( e );}))|| is(typeof( (R r, E e){r.opCall( (&e)[0..1] );})); else enum _putCall = is(typeof( (R r, E e){r( e );}))|| is(typeof( (R r, E e){r( (&e)[0..1] );})); }
Comment #6 by sandford — 2011-04-02T22:26:10Z
prefered -> preferred /** Outputs $(D e) to $(D r). The exact effect is dependent upon the two types. Several cases are accepted, as described below. The code snippets are attempted in order, and the first to compile "wins" and gets evaluated. Setting $(D asArray) to true will skip code snippets using user defined $(D put) and $(D opCall) methods, allowing templated user defined $(D put) methods to reuse some of $(D put)'s code snippets. Furthermore, $(D ER) allows the caller to set a preferred conversion type, enabling put to perform the correct UTF conversions for character and string inputs.
Comment #7 by sandford — 2012-02-14T20:48:55Z
I converted the patch into a pull request: https://github.com/D-Programming-Language/phobos/pull/433
Comment #8 by andrei — 2012-02-18T10:40:03Z
I think the patch has diverged from the intended charter of put, as mentioned in the pull request.
Comment #9 by sandford — 2012-03-19T09:56:04Z
(In reply to comment #8) > I think the patch has diverged from the intended charter of put, as mentioned > in the pull request. I am misunderstanding something. In the pull request, you mentioned that > The intent of put(r, e) is to provide a universal generic interface for putting stuff into stuff using a unified syntax. As such, adding policies and options works against that charter. We're better off defining independent functions such as putArray etc. However, put already contains policies for handling arrays, ranges, delegates, etc. All I've attempted to do, on that front, is to add all the variations (i.e. edge cases) on those concepts, so that generic code functions correctly. The other feature I've added is some external control of those policies, a feature which is a interim measure until UFC is working.
Comment #10 by monarchdodra — 2013-10-25T00:15:58Z
Is any of this still relevent? The unittest passes, all the implementations seem incredibly complicated... In any case, since I also rolled out my own implementation: https://github.com/D-Programming-Language/phobos/pull/1569 I believe this further simplifies how put works, while consolidating the way it works. Most importantly though, it allows for "on the fly" transcoding.
Comment #11 by monarchdodra — 2014-05-10T14:36:57Z
AFAIK, with my last "put" pull about 6 months ago, all issues (that I know of) are resolved. @ Andrei Alexandrescu : You are assigned to this issue. Is there anything left outstand IRT "put"? If not, can you close this as resolved?
Comment #12 by andrei — 2014-05-11T02:22:01Z
(In reply to monarchdodra from comment #11) > AFAIK, with my last "put" pull about 6 months ago, all issues (that I know > of) are resolved. > > @ Andrei Alexandrescu : You are assigned to this issue. Is there anything > left outstand IRT "put"? If not, can you close this as resolved? If you're fine, I'm fine. I've never gotten into this fully. Feel free to close if confident. Thanks!