Hello from France everyone ! (I hope my english will be readable...)

Yesterday, I discover D and that was really really great. I am far from being a guru (it seems there are a lot here ;-) but I foud the language clear and expressive the documentation is very well done futhermore dmd is incredibly fast and the logs are pure awesomness (a gcc command often doesn't fit on my shell). I know you are waiting for a 'but' and there is one : it's about operators overloading. First, but that kind of ok with me, I don't like the syntax :

	const Vertex opBinary(string op)(const ref Vertex vertex)
		if (op == "+" || op == "-")
	{
		Vertex result = *this;
		mixin("result" ~ op ~ "= vertex;");
		return result;
	}

I don't like the 'if' outside and the condition could be really long : if (op == "+" || op == "-" || op == "*" || op == "/"). Okay, putting the if inside is quite easy :

	const Vertex opBinary(string op)(const ref Vertex vertex)	
	{
		static if 	if (op == "+" || op == "-")
		{
			Vertex result = *this;
			mixin("result" ~ op ~ "= vertex;");
			return result;
		}
	}

But...I'd like to define cross product :

	const T opBinary(string op)(const ref Vertex vertex)
	{
		static if (op == "*")
		{
			T result = 0;
			foreach(i; 0..vertex_size)
				result += coordinate[i] * vertex.coordinate[i];
			return result;
		}
	}

The compiler is not so great here and doesn't distinguish the two opBinary by their signatures. Why ? Before looking at the documentation again I tried something like that :
	
	const Vertex opBinary(string op : ["+", "-"])(const ref Vertex vertex)	
	{
		{
			Vertex result = *this;
			mixin("result" ~ op ~ "= vertex;");
			return result;
		}
	}

	const T opBinary(string op : "*")(const ref Vertex vertex)
	{
		
		T result = 0;
		foreach(i; 0..vertex_size)
			result += coordinate[i] * vertex.coordinate[i];
		return result;
	}

The second one is okay, but the fisrt one isn't correct. I think this kind of template specialization over a list could be great with mixins and static if/switch. What do you think ? May be there is an other way to do what I want...

On Sunday, May 19, 2013 09:32:09 matovitch wrote:
> Hello from France everyone ! (I hope my english will be
> readable...)
> 
> Yesterday, I discover D and that was really really great. I am far from being a guru (it seems there are a lot here ;-) but I foud the language clear and expressive the documentation is very well done futhermore dmd is incredibly fast and the logs are pure awesomness (a gcc command often doesn't fit on my shell). I know you are waiting for a 'but' and there is one : it's about operators overloading. First, but that kind of ok with me, I don't like the syntax :
> 
> 	const Vertex opBinary(string op)(const ref Vertex vertex)
> 		if (op == "+" || op == "-")
> 	{
> 		Vertex result = *this;
> 		mixin("result" ~ op ~ "= vertex;");
> 		return result;
> 	}
> 
> I don't like the 'if' outside and the condition could be really long : if (op == "+" || op == "-" || op == "*" || op == "/"). Okay, putting the if inside is quite easy :
> 
> 	const Vertex opBinary(string op)(const ref Vertex vertex)
> 	{
> 		static if 	if (op == "+" || op == "-")
> 		{
> 			Vertex result = *this;
> 			mixin("result" ~ op ~ "= vertex;");
> 			return result;
> 		}
> 	}
> 
> But...I'd like to define cross product :
> 
> 	const T opBinary(string op)(const ref Vertex vertex)
> 	{
> 		static if (op == "*")
> 		{
> 			T result = 0;
> 			foreach(i; 0..vertex_size)
> 				result += coordinate[i] * vertex.coordinate[i];
> 			return result;
> 		}
> 	}
> 
> The compiler is not so great here and doesn't distinguish the two opBinary by their signatures. Why ? Before looking at the documentation again I tried something like that :
> 
> 	const Vertex opBinary(string op : ["+", "-"])(const ref Vertex
> vertex)
> 	{
> 		{
> 			Vertex result = *this;
> 			mixin("result" ~ op ~ "= vertex;");
> 			return result;
> 		}
> 	}
> 
> 	const T opBinary(string op : "*")(const ref Vertex vertex)
> 	{
> 
> 		T result = 0;
> 		foreach(i; 0..vertex_size)
> 			result += coordinate[i] * vertex.coordinate[i];
> 		return result;
> 	}
> 
> The second one is okay, but the fisrt one isn't correct. I think this kind of template specialization over a list could be great with mixins and static if/switch. What do you think ? May be there is an other way to do what I want...

If you want to distinguish between overloads of templated functions based on template parameters, template constraints are the correct way to do it. Nothing inside of the function - including static ifs - is considered in overloading. By the time the compiler even looks at that code, the overload has already been decided (or determined to be ambiguous). And what you can do directly in the template parameters to distinguish them - e.g. string op : "+" - is extremely limited. Much as you may not like it, the correct way to handle this is with template constraints. That's what they're for.

Now, there are ways to reduce their length - particularly if you're testing the same thing often. In that case, you could make an eponymous template for the test.

template isAddOrSub(string op)
{
    enum isAddOrSub = op == "+" || op == "-";
}

Then you can do

const Vertex opBinary(string op)(const ref Vertex vertex)
    if (isAddOrSub!op)
{...}

Now, in this particular case, I wouldn't think that it would be worth it. Which operators you need to use with a particular overload varies too much from type to type for an eponymous template like that to make much sense.

Another alternative would be to use a function like std.algorithm.equal except that it tested that one of the arguments was equal to the first one rather than just testing one argument against it. Something like

const Vertex opBinary(string op)(const ref Vertex vertex)
    if (isOneOf(op, "+", "-"))
{...}

Off the top of my head, I'm not aware of a function like that in the standard library, but I may just not being think of it, and it probably wouldn't be all that hard to write, at least if efficiency wasn't a major concern.

In general though, AFAIK, most people just do

const Vertex opBinary(string op)(const ref Vertex vertex)
    if (op == "+" || op == "-")
{...}

and don't have a problem with it.

- Jonathan M Davis

Fisrt, thank you for your clear answer.

On Sunday, 19 May 2013 at 09:04:23 UTC, Jonathan M Davis wrote:
> Nothing inside of the function - including static ifs - is considered in overloading.

I understand but it isn't about something which is inside the function : it is about its signature.

I agree that sometimes it might be useful to use more than a list of or, so the idea of template specilization over a list is stupid and theses template constraints are useful (even if I don't like the syntax, I couldn't imagine a better one).

On 05/19/2013 09:32 AM, matovitch wrote:
> Okay, putting the if inside is quite easy :
>

It is not a valid transformation. This removes the condition from the signature.

>      const Vertex opBinary(string op)(const ref Vertex vertex)
>      {
>          static /*[...]*/ if (op == "+" || op == "-")
>          {
>              Vertex result = *this;
>              mixin("result" ~ op ~ "= vertex;");
>              return result;
>          }
>      }
>
> But...I'd like to define cross product :
>
>      const T opBinary(string op)(const ref Vertex vertex)
>      {
>          static if (op == "*")
>          {
>              T result = 0;
>              foreach(i; 0..vertex_size)
>                  result += coordinate[i] * vertex.coordinate[i];
>              return result;
>          }
>      }
>
> The compiler is not so great here and doesn't distinguish the two
> opBinary by their signatures. Why ?

Because the two signatures are identical.

const T opBinary(string op)(const ref Vertex vertex);

Do this:

    const Vertex opBinary(string op)(const ref Vertex vertex)
    if (op == "+" || op == "-"){
        Vertex result = *this;
        mixin("result" ~ op ~ "= vertex;");
        return result;
    }

    const T opBinary(string op)(const ref Vertex vertex)
    if (op == "*"){
        T result = 0;
        foreach(i; 0..vertex_size)
            result += coordinate[i] * vertex.coordinate[i];
        return result;
    }

On Sunday, 19 May 2013 at 10:03:27 UTC, Timon Gehr wrote:
> On 05/19/2013 09:32 AM, matovitch wrote:
>> Okay, putting the if inside is quite easy :
>>
>
> It is not a valid transformation. This removes the condition from the signature.
>
>>     const Vertex opBinary(string op)(const ref Vertex vertex)
>>     {
>>         static /*[...]*/ if (op == "+" || op == "-")
>>         {
>>             Vertex result = *this;
>>             mixin("result" ~ op ~ "= vertex;");
>>             return result;
>>         }
>>     }
>>
>> But...I'd like to define cross product :
>>
>>     const T opBinary(string op)(const ref Vertex vertex)
>>     {
>>         static if (op == "*")
>>         {
>>             T result = 0;
>>             foreach(i; 0..vertex_size)
>>                 result += coordinate[i] * vertex.coordinate[i];
>>             return result;
>>         }
>>     }
>>
>> The compiler is not so great here and doesn't distinguish the two
>> opBinary by their signatures. Why ?
>
> Because the two signatures are identical.
>
> const T opBinary(string op)(const ref Vertex vertex);
>
> Do this:
>
>     const Vertex opBinary(string op)(const ref Vertex vertex)
>     if (op == "+" || op == "-"){
>         Vertex result = *this;
>         mixin("result" ~ op ~ "= vertex;");
>         return result;
>     }
>
>     const T opBinary(string op)(const ref Vertex vertex)
>     if (op == "*"){
>         T result = 0;
>         foreach(i; 0..vertex_size)
>             result += coordinate[i] * vertex.coordinate[i];
>         return result;
>     }

Oh yes ! You are right...it takes a vertex in both cases. Thanks for losing your time with me. :-)

Jonathan M Davis:

> Another alternative would be to use a function like std.algorithm.equal except
> that it tested that one of the arguments was equal to the first one rather than
> just testing one argument against it. Something like
>
> const Vertex opBinary(string op)(const ref Vertex vertex)
>     if (isOneOf(op, "+", "-"))
> {...}
>
> Off the top of my head, I'm not aware of a function like that in the standard
> library, but I may just not being think of it, and it probably wouldn't be all
> that hard to write, at least if efficiency wasn't a major concern.

It's named std.algorithm.canFind:

if (["+", "-"].canFind(op))

Bye,
bearophile

On Sunday, 19 May 2013 at 07:32:11 UTC, matovitch wrote:
> 	const Vertex opBinary(string op : ["+", "-"])(const ref Vertex vertex)	
> 	{
> 		{
> 			Vertex result = *this;
> 			mixin("result" ~ op ~ "= vertex;");
> 			return result;
> 		}
> 	}
>
> 	const T opBinary(string op : "*")(const ref Vertex vertex)
> 	{
> 		
> 		T result = 0;
> 		foreach(i; 0..vertex_size)
> 			result += coordinate[i] * vertex.coordinate[i];
> 		return result;
> 	}
>
> The second one is okay, but the fisrt one isn't correct. I think this kind of template specialization over a list could be great with mixins and static if/switch. What do you think ? May be there is an other way to do what I want...

I think a better syntax would be:

    const Vertex opBinary(string op : "+", "-")(const ref Vertex vertex)

this will save a bit of ambiguity(`op` is a `string` specialization of `string[]`?)

At any rate, I'm not sure how useful it would be. Operator overloading code usually fit for a single operator - you usually write one for each operator:

	const Vertex opBinary(string op : "+")(const ref Vertex vertex)
	{
		Vertex result = this;
        result += vertex;
		return result;
	}
	const Vertex opBinary(string op : "-")(const ref Vertex vertex)
	{
		Vertex result = this;
        result -= vertex;
		return result;
	}

If you want to have several operators in the same method, you can afford the verbosity of:

    const Vertex opBinary(string op)(const ref Vertex vertex)
        if (__traits(compiles, mixin("this " ~ op ~ "= vertex;")))
    {
        Vertex result = *this;
        mixin("result" ~ op ~ "= vertex;");
        return result;
    }

On Sunday, May 19, 2013 12:36:00 bearophile wrote:
> Jonathan M Davis:
> > Another alternative would be to use a function like
> > std.algorithm.equal except
> > that it tested that one of the arguments was equal to the first
> > one rather than
> > just testing one argument against it. Something like
> > 
> > const Vertex opBinary(string op)(const ref Vertex vertex)
> > 
> >     if (isOneOf(op, "+", "-"))
> > 
> > {...}
> > 
> > Off the top of my head, I'm not aware of a function like that
> > in the standard
> > library, but I may just not being think of it, and it probably
> > wouldn't be all
> > that hard to write, at least if efficiency wasn't a major
> > concern.
> 
> It's named std.algorithm.canFind:
> 
> if (["+", "-"].canFind(op))

Ah, yes. I was thinking backwards. At first, I was thinking canFind(op, "+", "-"), which definitely doesn't do the right thing, and for some reason, it didn't occur to me to flip it. I've probably been up too long and should get to bed.

- Jonathan M Davis

On Sunday, May 19, 2013 12:41:52 Idan Arye wrote:
> At any rate, I'm not sure how useful it would be. Operator overloading code usually fit for a single operator - you usually write one for each operator:

Actually, it's usually considered good practice to use the same function for several operators and mixin the operator. In most cases, with arithmetic operators, that allows you to use one function for several operators and is the main reason why overloaded operators now use strings like they do rather than being opAdd, opSub, etc. like they used to be. Sometimes, it _is_ necessary to have an overload per operator, but usually that can be avoided.

- Jonathan M Davis

On 05/19/2013 12:41 PM, Idan Arye wrote:
> ...
>
> I think a better syntax would be:
>
>      const Vertex opBinary(string op : "+", "-")(const ref Vertex vertex)
>
> this will save a bit of ambiguity(`op` is a `string` specialization of
> `string[]`?)
>

It moves the ambiguity from analysis into the parser.

> At any rate, I'm not sure how useful it would be. Operator overloading
> code usually fit for a single operator - you usually write one for each
> operator:
>
>      const Vertex opBinary(string op : "+")(const ref Vertex vertex)
>      {
>          Vertex result = this;
>          result += vertex;
>          return result;
>      }
>      const Vertex opBinary(string op : "-")(const ref Vertex vertex)
>      {
>          Vertex result = this;
>          result -= vertex;
>          return result;
>      }
>

Your example shows how operator overloading code often fits for multiple operators.

> If you want to have several operators in the same method, you can afford
> the verbosity of:
>
>      const Vertex opBinary(string op)(const ref Vertex vertex)
>          if (__traits(compiles, mixin("this " ~ op ~ "= vertex;")))
>      {
>          Vertex result = /*[...]*/this;
>          mixin("result" ~ op ~ "= vertex;");
>          return result;
>      }

This kind of template constraint may allow leaking of implementation details.

Eg. v.opBinary!".foo+"