I tried to post this last night, but the NG wasn't having any of it.

I found myself writing a bug that looked like this

    match(ts, TokenType.Is);
    match(ts, TokenType.OpenParen);
    isExp.type == parseType(ts);

The bug being of course, that a type is parsed and ts is modified,
so the expression has side effects so it's not flagged as a useless
expression. But the comparison still has no effect, so should this be
flagged by DMD?

-Bernard.

Bernard Helyer:

>     match(ts, TokenType.Is);
>     match(ts, TokenType.OpenParen);
>     isExp.type == parseType(ts);
>
> The bug being of course, that a type is parsed and ts is modified, so the expression has side effects so it's not
> flagged as a useless expression. But the comparison still
> has no effect, so should this be flagged by DMD?

The top level operation is the ==, that is a pure expression, so maybe dmd should warn on this.

Bye,
bearophile

On Tuesday, 25 September 2012 at 19:29:26 UTC, Bernard Helyer wrote:
> I tried to post this last night, but the NG wasn't having any of it.
>
> I found myself writing a bug that looked like this
>
>     match(ts, TokenType.Is);
>     match(ts, TokenType.OpenParen);
>     isExp.type == parseType(ts);
>
> The bug being of course, that a type is parsed and ts is modified,
> so the expression has side effects so it's not flagged as a useless
> expression.

Err, the bug is that I wrote '==' instead of '='. That's what
I get for posting first thing in the morning, I guess.

On Tuesday, 25 September 2012 at 21:15:24 UTC, bearophile wrote:
> The top level operation is the ==, that is a pure expression, so maybe dmd should warn on this.

Yeah, I thought so but I'm not sure. Hence me asking.

On 25/09/12 21:30, Bernard Helyer wrote:
> I tried to post this last night, but the NG wasn't having any of it.
>
> I found myself writing a bug that looked like this
>
>      match(ts, TokenType.Is);
>      match(ts, TokenType.OpenParen);
>      isExp.type == parseType(ts);
>
> The bug being of course, that a type is parsed and ts is modified,
> so the expression has side effects so it's not flagged as a useless
> expression. But the comparison still has no effect, so should this be
> flagged by DMD?
>
> -Bernard.


The "must have an effect" rule only applies to statements, not expressions, so this is according to the spec. It's not a bug.

This is a bit like the more extreme case I recently posted about:

int x, y, z;
x == y, ++z;

doesn't generate an error message even though x == y has no side-effects, because comma is an expression, not a statement.

IMHO, every expression should be required to have an effect. For example
foo() + foo();
shouldn't compile, unless + is an overloaded operator with side-effects.

It would be really interesting to add this check, and then compile existing code (such as Phobos) to see if it breaks valid code, or reveals bugs.

On 09/26/2012 11:45 AM, Don Clugston wrote:
> On 25/09/12 21:30, Bernard Helyer wrote:
>> I tried to post this last night, but the NG wasn't having any of it.
>>
>> I found myself writing a bug that looked like this
>>
>>      match(ts, TokenType.Is);
>>      match(ts, TokenType.OpenParen);
>>      isExp.type == parseType(ts);
>>
>> The bug being of course, that a type is parsed and ts is modified,
>> so the expression has side effects so it's not flagged as a useless
>> expression. But the comparison still has no effect, so should this be
>> flagged by DMD?
>>
>> -Bernard.
>
>
> The "must have an effect" rule only applies to statements, not
> expressions, so this is according to the spec. It's not a bug.
>
> This is a bit like the more extreme case I recently posted about:
>
> int x, y, z;
> x == y, ++z;
>
> doesn't generate an error message even though x == y has no
> side-effects, because comma is an expression, not a statement.
>
> IMHO, every expression should be required to have an effect.

I'd rather restrict this to expressions whose value is unused.
Otherwise 'a = b is c;' will be illegal, because 'b is c' does not have
an effect.

> For example foo() + foo();
> shouldn't compile, unless + is an overloaded operator

Yes.

> with side-effects.
>

Not sure about that. In generic code, it is possible and likely (as pure is inferred for lambdas and template functions) that a pure
function ends up being called for potential side effects.

> It would be really interesting to add this check, and then compile
> existing code (such as Phobos) to see if it breaks valid code, or
> reveals bugs.
>

On 26/09/12 14:19, Timon Gehr wrote:
> On 09/26/2012 11:45 AM, Don Clugston wrote:
>> On 25/09/12 21:30, Bernard Helyer wrote:
>>> I tried to post this last night, but the NG wasn't having any of it.
>>>
>>> I found myself writing a bug that looked like this
>>>
>>>      match(ts, TokenType.Is);
>>>      match(ts, TokenType.OpenParen);
>>>      isExp.type == parseType(ts);
>>>
>>> The bug being of course, that a type is parsed and ts is modified,
>>> so the expression has side effects so it's not flagged as a useless
>>> expression. But the comparison still has no effect, so should this be
>>> flagged by DMD?
>>>
>>> -Bernard.
>>
>>
>> The "must have an effect" rule only applies to statements, not
>> expressions, so this is according to the spec. It's not a bug.
>>
>> This is a bit like the more extreme case I recently posted about:
>>
>> int x, y, z;
>> x == y, ++z;
>>
>> doesn't generate an error message even though x == y has no
>> side-effects, because comma is an expression, not a statement.
>>
>> IMHO, every expression should be required to have an effect.
>
> I'd rather restrict this to expressions whose value is unused.

If the value is used, it has an effect.

> Otherwise 'a = b is c;' will be illegal, because 'b is c' does not have
> an effect.
>
>> For example foo() + foo();
>> shouldn't compile, unless + is an overloaded operator
>
> Yes.
>
>> with side-effects.
>>
>
> Not sure about that. In generic code, it is possible and likely (as pure
> is inferred for lambdas and template functions) that a pure
> function ends up being called for potential side effects.

I wouldn't give it special treatment. I think it should be allowed if and only if

plus(foo(), foo());

compiles, when plus is pure nothrow.

Timon Gehr:

> In generic code, it is possible and likely (as pure is inferred for lambdas and template functions) that a pure function ends up being called for potential side effects.

Already tried that :-(

Bye,
bearophile

On 26/09/2012 10:45, Don Clugston wrote:
> On 25/09/12 21:30, Bernard Helyer wrote:
>> I tried to post this last night, but the NG wasn't having any of it.
>>
>> I found myself writing a bug that looked like this
>>
>>      match(ts, TokenType.Is);
>>      match(ts, TokenType.OpenParen);
>>      isExp.type == parseType(ts);
>>
>> The bug being of course, that a type is parsed and ts is modified,
>> so the expression has side effects so it's not flagged as a useless
>> expression. But the comparison still has no effect, so should this be
>> flagged by DMD?
>>
>> -Bernard.
>
>
> The "must have an effect" rule only applies to statements, not
> expressions, so this is according to the spec. It's not a bug.
>
> This is a bit like the more extreme case I recently posted about:
>
> int x, y, z;
> x == y, ++z;
>
> doesn't generate an error message even though x == y has no
> side-effects, because comma is an expression, not a statement.
>
> IMHO, every expression should be required to have an effect. For example
> foo() + foo();
> shouldn't compile, unless + is an overloaded operator with side-effects.

What's a realistic use case for allowing foo() + foo(), even if it is overloaded to have side-effects? It looks like an expression with no side-effects. Surely if anyone actually intends it to have side-effects, then they've made a fundamental design mistake in their API?

I know C++ has << used this way, but that's a bit horrible.