Bug 1968 – boxer.d does not work

Status
RESOLVED
Resolution
WONTFIX
Severity
normal
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
PowerPC
OS
Mac OS X
Creation time
2008-04-01T10:41:00Z
Last change time
2015-06-09T05:14:42Z
Assigned to
dvdfrdmn
Creator
fawzi
Depends on
2004

Comments

Comment #0 by fawzi — 2008-04-01T10:41:18Z
Doing my tangobos merge I came across a bug in std.boxer. Actually in phobos it passes the unit tests, but is still broken. The issue can be seen with the following program ========== import std.stdio; import std.boxer; void main() { auto array=boxArray("ab","cd","ef"); assert(unbox!(char[])(array[0])=="ab"); assert(unbox!(char[])(array[1])=="cd"); assert(unbox!(char[])(array[2])=="ef"); writefln(unbox!(char[])(array[2])); } ======== The main issue is that the generic handling does not update _argptr. doing a modification like this - array[index] = box(ti_orig, cast(void*) _argptr); + void *v=cast(void*)_argptr; + array[index] = box(ti_orig, v); + // update _argptr + v+=ti.tsize; + void **vv=cast(void**)&_argptr; + *vv=v; fixes the problem, at least on MacOsX. Still this is a dangerous operation because it makes assumptions on _argptr (that from now on the arguments are on the stack and a pointer to them is the first element in va_list). So I also added to the function a lot of special cases (all basic types, arrays of them and all pointer types) in the dumb way (by typing them out). I think that this way it should be rather safe, and work in most occasions. Here is the patch wit a slight change in the import and an extra cast. --- boxer.d (revision 209) +++ boxer.d (working copy) @@ -68,10 +68,9 @@ module std.boxer; private import std.format; +import std.stdarg; private import std.string; private import std.utf; -version (GNU) - private import std.stdarg; /* These functions and types allow packing objects into generic containers * and recovering them later. This comes into play in a wide spectrum of @@ -502,7 +501,7 @@ /** * Box each argument passed to the function, returning an array of boxes. - */ + */ Box[] boxArray(...) { version (GNU) @@ -517,28 +516,128 @@ while ( (ttd = cast(TypeInfo_Typedef) ti) !is null ) ti = ttd.base; - if (ti is typeid(float)) + if (ti is typeid(bool)) { - float f = va_arg!(float)(_argptr); - array[index] = box(ti_orig, cast(void *) & f); + auto a = va_arg!(bool)(_argptr); + array[index] = box(ti_orig, cast(void *) & a); } else if (ti is typeid(char) || ti is typeid(byte) || ti is typeid(ubyte)) { - byte b = va_arg!(byte)(_argptr); - array[index] = box(ti_orig, cast(void *) & b); + auto a = va_arg!(byte)(_argptr); + array[index] = box(ti_orig, cast(void *) & a); } - else if (ti is typeid(wchar) || ti is typeid(short) || ti is typeid(ushort)) + else if (ti is typeid(wchar)) { - short s = va_arg!(short)(_argptr); - array[index] = box(ti_orig, cast(void *) & s); + auto a = va_arg!(wchar)(_argptr); + array[index] = box(ti_orig, cast(void *) & a); } - else if (ti is typeid(bool)) + else if (ti is typeid(short) || ti is typeid(ushort)) { - bool b = va_arg!(bool)(_argptr); - array[index] = box(ti_orig, cast(void *) & b); + auto a = va_arg!(short)(_argptr); + array[index] = box(ti_orig, cast(void *) & a); } - else - array[index] = box(ti_orig, cast(void*) _argptr); + else if (ti is typeid(int) || ti is typeid(uint)) + { + auto a = va_arg!(int)(_argptr); + array[index] = box(ti_orig, cast(void *) & a); + } + else if (ti is typeid(long) || ti is typeid(ulong)) + { + auto a = va_arg!(long)(_argptr); + array[index] = box(ti_orig, cast(void *) & a); + } + else if (ti is typeid(float) || ti is typeid(ifloat)) + { + auto a = va_arg!(float)(_argptr); + array[index] = box(ti_orig, cast(void *) & a); + } + else if (ti is typeid(double) || ti is typeid(idouble)) + { + auto a = va_arg!(double)(_argptr); + array[index] = box(ti_orig, cast(void *) & a); + } + else if (ti is typeid(real) || ti is typeid(ireal)) + { + auto a = va_arg!(real)(_argptr); + array[index] = box(ti_orig, cast(void *) & a); + } + else if (ti is typeid(bool[])) + { + auto a = va_arg!(bool[])(_argptr); + array[index] = box(ti_orig, cast(void *) & a); + } + else if (ti is typeid(char[]) || ti is typeid(byte[]) || ti is typeid(ubyte[])) + { + auto a = va_arg!(byte[])(_argptr); + array[index] = box(ti_orig, cast(void *) & a); + } + else if (ti is typeid(wchar[])) + { + auto a = va_arg!(wchar[])(_argptr); + array[index] = box(ti_orig, cast(void *) & a); + } + else if (ti is typeid(short[]) || ti is typeid(ushort[])) + { + auto a = va_arg!(short[])(_argptr); + array[index] = box(ti_orig, cast(void *) & a); + } + else if (ti is typeid(int[]) || ti is typeid(uint[])) + { + auto a = va_arg!(int[])(_argptr); + array[index] = box(ti_orig, cast(void *) & a); + } + else if (ti is typeid(long[]) || ti is typeid(ulong[])) + { + auto a = va_arg!(long[])(_argptr); + array[index] = box(ti_orig, cast(void *) & a); + } + else if (ti is typeid(float[]) || ti is typeid(ifloat[])) + { + auto a = va_arg!(float[])(_argptr); + array[index] = box(ti_orig, cast(void *) & a); + } + else if (ti is typeid(double[]) || ti is typeid(idouble[])) + { + auto a = va_arg!(double[])(_argptr); + array[index] = box(ti_orig, cast(void *) & a); + } + else if (ti is typeid(real[]) || ti is typeid(ireal[])) + { + auto a = va_arg!(real[])(_argptr); + array[index] = box(ti_orig, cast(void *) & a); + } + else if (ti is typeid(cfloat[])) + { + auto a = va_arg!(cfloat[])(_argptr); + array[index] = box(ti_orig, cast(void *) & a); + } + else if (ti is typeid(cdouble[])) + { + auto a = va_arg!(cdouble[])(_argptr); + array[index] = box(ti_orig, cast(void *) & a); + } + else if (ti is typeid(creal[])) + { + auto a = va_arg!(creal[])(_argptr); + array[index] = box(ti_orig, cast(void *) & a); + } + else if (ti.tsize == (void *).sizeof) + { // treats all pointer based cases + void *a = va_arg!(void *)(_argptr); + array[index] = box(ti_orig, cast(void *) & a); + } + else + { + // structures passed on the stack + // assumes a stack based storage, and that the pointer to it + // can be retrived via cast... + void *v=cast(void*)_argptr; + array[index] = box(ti_orig, v); + // update _argptr + v+=ti.tsize; + void **vv=cast(void**)&_argptr; // this is dangerous... + *vv=v; // ... very dangerous... but seems to work ;) + } } return array; @@ -797,7 +896,7 @@ if (isArrayTypeInfo(value.type)) return (*cast(void[]*) value.data).ptr; if (cast(TypeInfo_Class) value.type) - return *cast(Object*) value.data; + return cast(T)(*cast(Object*) value.data); throw new UnboxException(value, typeid(T)); }
Comment #1 by fawzi — 2008-04-01T10:46:17Z
I do not know if the update of _argptr + v+=ti.tsize; should take into account alignement issues. Probably it should... Fawzi
Comment #2 by mike.kinney — 2009-07-16T23:02:49Z
Perl http://www.digitalmars.com/d/2.0/phobos/std_boxer.html: WARNING: This module is being phased out. You may want to use std.variant for new code. Ok to close this (really old) issue?
Comment #3 by fawzi — 2009-07-17T00:04:54Z
Yes for me it is ok, actually I don't use it... (switched to tango). I think that not fixing a deprecated module makes perfectly sense.