View mode: basic / threaded / horizontal-split · Log in · Help
May 24, 2012
[Issue 8143] New: Safe std.conv.to enum conversion
http://d.puremagic.com/issues/show_bug.cgi?id=8143

          Summary: Safe std.conv.to enum conversion
          Product: D
          Version: D2
         Platform: All
       OS/Version: All
           Status: NEW
         Severity: enhancement
         Priority: P2
        Component: Phobos
       AssignedTo: nobody@puremagic.com
       ReportedBy: bearophile_hugs@eml.cc


--- Comment #0 from bearophile_hugs@eml.cc 2012-05-24 14:46:34 PDT ---
import std.conv: to;
enum Foo : int { A = 10, B = 20 }
void main() {
   int x = 10;
   Foo f1 = to!Foo(x);
   assert(f1 == Foo.A);
   x = 5;
   Foo f2 = to!Foo(x); // needs to throw an exception
}


DMD 2.060alpha gives:

...\dmd2\src\phobos\std\conv.d(267): Error: template std.conv.toImpl does not
match any function template declaration
...\dmd2\src\phobos\std\conv.d(298): Error: template std.conv.toImpl cannot
deduce template function from argument types !(Foo)(int)
...\dmd2\src\phobos\std\conv.d(267): Error: template instance toImpl!(Foo)
errors instantiating template
temp.d(5): Error: template instance std.conv.to!(Foo).to!(int) error
instantiating


This is handy to *safely* convert run-time values to enum items. Using a
cast(Foo) is useful in other cases, because cast(Foo) doesn't raise a run-time
exceptions. The same difference is visible in this program:


import std.conv: to;
void main() {
   int x = -10;
   uint y1 = cast(uint)x; // no errors here
   uint y2 = to!uint(x); // throws std.conv.ConvOverflowException
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
October 23, 2012
[Issue 8143] Safe std.conv.to enum conversion
http://d.puremagic.com/issues/show_bug.cgi?id=8143


Andrej Mitrovic <andrej.mitrovich@gmail.com> changed:

          What    |Removed                     |Added
----------------------------------------------------------------------------
                CC|                            |andrej.mitrovich@gmail.com


--- Comment #1 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2012-10-23 16:44:27 PDT ---
One problem with this is:

   enum EF : float { C = 4.9 }
   float f = 4.9;
   EF en2 = to!EF(f);

This will fail internally if conv.to compares members via "==", because of
floating point comparison semantics.

So the question is, is this going to be a problem? If yes, should we use
approxEqual for floating point comparisons? Or maybe we should simply ban using
std.conv on enums that have a floating point base type?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
October 24, 2012
[Issue 8143] Safe std.conv.to enum conversion
http://d.puremagic.com/issues/show_bug.cgi?id=8143



--- Comment #2 from bearophile_hugs@eml.cc 2012-10-23 18:59:14 PDT ---
(In reply to comment #1)

> This will fail internally if conv.to compares members via "==", because of
> floating point comparison semantics.
> 
> So the question is, is this going to be a problem? If yes, should we use
> approxEqual for floating point comparisons?

By far the main purpose of enums is with integral values (ints, uint, chars,
etc), to be used to enumerate something or as bitfields. Using
float/double/real enums is supported in D, but it's not common.

Using approxEqual is suboptimal, using std.math.feqrel is better. but all
approximate floating point comparisons have their quirks and limits. Backing-in
one solution is not a good idea.


> Or maybe we should simply ban using
> std.conv on enums that have a floating point base type?

What about user-defined floating point types, or a double wrapped in a struct
with an alias this?

I think refusing conv on built-in floating point types is an acceptable
solution to avoid most troubles. Other cases like wrapped doubles are left at
the care of the programmer.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
October 24, 2012
[Issue 8143] Safe std.conv.to enum conversion
http://d.puremagic.com/issues/show_bug.cgi?id=8143


monarchdodra@gmail.com changed:

          What    |Removed                     |Added
----------------------------------------------------------------------------
                CC|                            |monarchdodra@gmail.com


--- Comment #3 from monarchdodra@gmail.com 2012-10-24 07:58:55 PDT ---
(In reply to comment #2)
> (In reply to comment #1)
> > This will fail internally if conv.to compares members via "==", because of
> > floating point comparison semantics.
> > 
> > So the question is, is this going to be a problem? If yes, should we use
> > approxEqual for floating point comparisons?
> By far the main purpose of enums is with integral values (ints, uint, chars,
> etc), to be used to enumerate something or as bitfields. Using
> float/double/real enums is supported in D, but it's not common.
> Using approxEqual is suboptimal, using std.math.feqrel is better. but all
> approximate floating point comparisons have their quirks and limits. Backing-in
> one solution is not a good idea.
> > Or maybe we should simply ban using
> > std.conv on enums that have a floating point base type?
> What about user-defined floating point types, or a double wrapped in a struct
> with an alias this?
> I think refusing conv on built-in floating point types is an acceptable
> solution to avoid most troubles. Other cases like wrapped doubles are left at
> the care of the programmer.

I'd say there is nothing wrong with using floats as enums. It's rare because
it's new (C++ only supported integral up to now). In C++, I've seen integral
based enums used to index arrays which contained the actual payload (floats,
strigns, others). Now we don't have to do this anymore.

The *real* issue (IMO) is only when converting *back* from float to enum, which
(IMO again), is plain too dangerous to realisticly assume we can support.

I'd rather have float-to-enum always fail, personally.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
October 24, 2012
[Issue 8143] Safe std.conv.to enum conversion
http://d.puremagic.com/issues/show_bug.cgi?id=8143



--- Comment #4 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2012-10-24 08:02:31 PDT ---
Or alternatively require an additional alias parameter for the comparison
function in case of floats?

E.g.

   enum EF : float { C = 4.9 }
   float f = 4.9;
   static bool compFunc(float lhs, float rhs) { ... }
   EF en2 = to!(EF, compFunc)(f);

Since conv.to will already do the work necessary to 1) find the matching
member, 2) ensure there's only 1 matching member, I think customization like
this might be friendlier than just rejecting conversion.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
October 24, 2012
[Issue 8143] Safe std.conv.to enum conversion
http://d.puremagic.com/issues/show_bug.cgi?id=8143


Andrej Mitrovic <andrej.mitrovich@gmail.com> changed:

          What    |Removed                     |Added
----------------------------------------------------------------------------
        AssignedTo|nobody@puremagic.com        |andrej.mitrovich@gmail.com


--- Comment #5 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2012-10-24 10:13:51 PDT ---
Ok here's a first implementation, let me know if it can be improved before a
pull request is made (the docs will be improved too):

External: http://dpaste.dzfl.pl/ee99ce99
And copied here:

import std.traits;
import std.conv : ConvException, assertThrown;
import std.string;
import std.math;
import std.stdio;

/**
Convert a value that is implicitly convertible to the enum base type
into an Enum value.
If the value does not match any enum member values, or
if it matches more than one member value throw a ConvException.
*/
T toImpl(T, S)(S value)
   if (is(T == enum) && is(S : OriginalType!T) &&
!isFloatingPoint!(OriginalType!T))
{
   T result;
   size_t matches;

   foreach (Member; EnumMembers!T)
   {
       if (Member == value)
       {
           result = Member;
           if (++matches > 1)
               throw new ConvException(format("Value (%s) matches more than
one member value of enum '%s'", value, fullyQualifiedName!T));
       }
   }

   if (!matches)
       throw new ConvException(format("Value (%s) does not match any member
value of enum '%s'", value, fullyQualifiedName!T));

   return result;
}

/**
   Ditto:

   Specialization for Enums that have a floating point base type.
   @equal must be a function which takes the enum base type as
   its first parameter, the type of @value as its second parameter,
   and return true if the two compare equal.
*/
T toImpl(T, alias equal, S)(S value)
   if (is(T == enum) && is(S : OriginalType!T) &&
isFloatingPoint!(OriginalType!T))
{
   T result;
   size_t matches;

   foreach (Member; EnumMembers!T)
   {
       if (equal(Member, value))
       {
           result = Member;
           if (++matches > 1)
               throw new ConvException(format("Value (%s) matches more than
one member value of enum '%s'", value, fullyQualifiedName!T));
       }
   }

   if (!matches)
       throw new ConvException(format("Value (%s) does not match any member
value of enum '%s'", value, fullyQualifiedName!T));

   return result;
}

alias toImpl to;

void test()
{
   enum En : int { A = 10, B = 20, C = 30, D = 20 }
   En en1 = to!En(10);
   assert(en1 == En.A);
   assertThrown!ConvException(to!En(5));
   // matches more than one
   assertThrown!ConvException(to!En(20));

   static bool equal(float a, float b) { return feqrel(a, b) >= 24; }
   enum EF : float { C = 4.9 }
   float f = 4.9;
   EF enf = to!(EF, equal)(f);

   enum EF2 : float { A = 4.9, B = 1.0, C = 4.9 }
   float f2 = 4.9;
   // matches more than one
   assertThrown!ConvException(to!(EF2, equal)(f2));
}

void main()
{
   test();
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
October 24, 2012
[Issue 8143] Safe std.conv.to enum conversion
http://d.puremagic.com/issues/show_bug.cgi?id=8143


bearophile_hugs@eml.cc changed:

          What    |Removed                     |Added
----------------------------------------------------------------------------
        AssignedTo|andrej.mitrovich@gmail.com  |nobody@puremagic.com


--- Comment #6 from bearophile_hugs@eml.cc 2012-10-24 10:19:48 PDT ---
(In reply to comment #4)

> I think customization like
> this might be friendlier than just rejecting conversion.

It also makes the implementation and usage a bit more complex. Ask to other
people (like Andrei) to see what they think. (Here my preference goes to a
simple solution).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
October 24, 2012
[Issue 8143] Safe std.conv.to enum conversion
http://d.puremagic.com/issues/show_bug.cgi?id=8143



--- Comment #7 from bearophile_hugs@eml.cc 2012-10-24 10:26:36 PDT ---
(In reply to comment #5)
> Ok here's a first implementation, let me know if it can be improved before a
> pull request is made (the docs will be improved too):

One more test case:

En[][] m1 = to!(En[][])([[10, 30], [30, 10]]);


Have you compiled your code with "-property -w"?

I am seeing some errors:

...\dmd2\src\phobos\std\traits.d(221): Error: not a property test
...\dmd2\src\phobos\std\traits.d(225): Error: not a property test
...\dmd2\src\phobos\std\traits.d(229): Error: not a property test
...\dmd2\src\phobos\std\traits.d(234): Error: not a property test
...\dmd2\src\phobos\std\traits.d(231): Error: template instance
std.traits.fullyQualifiedName!(test) error instantiating
test.d(27):        instantiated from here: fullyQualifiedName!(En)
test.d(76):        instantiated from here: toImpl!(En,int)
test.d(27): Error: template instance std.traits.fullyQualifiedName!(En) error
instantiating
test.d(76):        instantiated from here: toImpl!(En,int)
test.d(26): Error: constructor std.conv.ConvException.this (string s, string fn
= __FILE__, uint ln = cast(uint)__LINE__) is not callable using argument types
(_error_)
test.d(26): Error: constructor std.conv.ConvException.this (string s, string fn
= __FILE__, uint ln = cast(uint)__LINE__) is not callable using argument types
(_error_)
test.d(26): Error: constructor std.conv.ConvException.this (string s, string fn
= __FILE__, uint ln = cast(uint)__LINE__) is not callable using argument types
(_error_)
test.d(26): Error: constructor std.conv.ConvException.this (string s, string fn
= __FILE__, uint ln = cast(uint)__LINE__) is not callable using argument types
(_error_)
test.d(32): Error: constructor std.conv.ConvException.this (string s, string fn
= __FILE__, uint ln = cast(uint)__LINE__) is not callable using argument types
(_error_)
test.d(76): Error: template instance test.toImpl!(En,int) error instantiating

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
October 24, 2012
[Issue 8143] Safe std.conv.to enum conversion
http://d.puremagic.com/issues/show_bug.cgi?id=8143



--- Comment #8 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2012-10-24 10:28:51 PDT ---
(In reply to comment #6)
> (In reply to comment #4)
> 
> > I think customization like
> > this might be friendlier than just rejecting conversion.
> 
> It also makes the implementation and usage a bit more complex. Ask to other
> people (like Andrei) to see what they think. (Here my preference goes to a
> simple solution).

It's only complex for the case of floating point conversion. We could by
default set the alias to be a safe floating-point comparison function by
default so the user doesn't have to pass one if he doesn't want to.

There are other to!() implementations that take special arguments, e.g. in
radix conversions an extra argument is passed.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
October 24, 2012
[Issue 8143] Safe std.conv.to enum conversion
http://d.puremagic.com/issues/show_bug.cgi?id=8143



--- Comment #9 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2012-10-24 10:31:14 PDT ---
(In reply to comment #7)
> One more test case:
> En[][] m1 = to!(En[][])([[10, 30], [30, 10]]);

Ah, haven't thought about arrays. Will fix..

> Have you compiled your code with "-property -w"?
> I am seeing some errors:

Those seem to be Phobos errors, unrelated to my code.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
« First   ‹ Prev
1 2 3
Top | Discussion index | About this forum | D home