Jump to page: 1 2 3
Thread overview
comma operator causes hard to spot bugs
Apr 21, 2012
Benjamin Thaut
Apr 21, 2012
Jonathan M Davis
Apr 21, 2012
deadalnix
Apr 21, 2012
bearophile
Apr 21, 2012
Timon Gehr
Apr 21, 2012
Christophe
Apr 22, 2012
deadalnix
Apr 22, 2012
Andrej Mitrovic
Apr 24, 2012
Martin Nowak
Apr 24, 2012
Timon Gehr
Apr 22, 2012
deadalnix
Apr 23, 2012
Timon Gehr
Apr 23, 2012
CTFE-4-the-win
Apr 28, 2012
Era Scarecrow
Apr 28, 2012
Era Scarecrow
Apr 21, 2012
Artur Skawina
Apr 27, 2012
Norbert Nemec
Apr 27, 2012
Manfred Nowak
Apr 27, 2012
David Nadlinger
Apr 28, 2012
Manfred Nowak
Apr 28, 2012
David Nadlinger
Apr 28, 2012
Manfred Nowak
April 21, 2012
Hi,
I just had a bug due to the unitentional usage of the comma operator. So I'm wondering if there could something be done so that the compiler prevents something like this:

memStart = cast(T*)(m_allocator.AllocateMemory(size * T.sizeof), InitializeMemoryWith.NOTHING);

This first excutes AllocateMemory, then throws away the result and then casts the enum InitializeMemory.NOTHING value to a pointer, which will always result in a null pointer. This took me quite some time to spot.
What I actually wanted to do:

memStart = cast(T*)(m_allocator.AllocateMemory(size * T.sizeof, InitializeMemoryWith.NOTHING));

1) So is it actually neccessary that enums can be casted directly to pointers?
2) Is the functionality provided by the comma operator actually worth the bugs it causes?

Kind Regards
Benjamin Thaut
April 21, 2012
On 21-04-2012 14:23, Benjamin Thaut wrote:
> Hi,
> I just had a bug due to the unitentional usage of the comma operator. So
> I'm wondering if there could something be done so that the compiler
> prevents something like this:
>
> memStart = cast(T*)(m_allocator.AllocateMemory(size * T.sizeof),
> InitializeMemoryWith.NOTHING);
>
> This first excutes AllocateMemory, then throws away the result and then
> casts the enum InitializeMemory.NOTHING value to a pointer, which will
> always result in a null pointer. This took me quite some time to spot.
> What I actually wanted to do:
>
> memStart = cast(T*)(m_allocator.AllocateMemory(size * T.sizeof,
> InitializeMemoryWith.NOTHING));
>
> 1) So is it actually neccessary that enums can be casted directly to
> pointers?
> 2) Is the functionality provided by the comma operator actually worth
> the bugs it causes?

No. It's an abomination, and it blocks more novel/interesting language design opportunities.

>
> Kind Regards
> Benjamin Thaut


-- 
- Alex
April 21, 2012
On 04/21/12 14:23, Benjamin Thaut wrote:
> Hi,
> I just had a bug due to the unitentional usage of the comma operator. So I'm wondering if there could something be done so that the compiler prevents something like this:
> 
> memStart = cast(T*)(m_allocator.AllocateMemory(size * T.sizeof), InitializeMemoryWith.NOTHING);
> 
> This first excutes AllocateMemory, then throws away the result and then casts the enum InitializeMemory.NOTHING value to a pointer, which will always result in a null pointer. This took me quite some time to spot.
> What I actually wanted to do:
> 
> memStart = cast(T*)(m_allocator.AllocateMemory(size * T.sizeof, InitializeMemoryWith.NOTHING));
> 
> 1) So is it actually neccessary that enums can be casted directly to pointers?
> 2) Is the functionality provided by the comma operator actually worth the bugs it causes?

Probably, but it *is* rarely used...

However the problem is the allocator, which should also take the type is an
argument and do the cast internally. That plus an overload for arrays would avoid
these kinds of bugs (since an API like this will be enough for most use cases).
And, yes, the D std runtime got this wrong too.
(The void*alloc(size_t,flags) versions are useful sometimes, of course)

artur
April 21, 2012
On Saturday, April 21, 2012 14:25:14 Alex Rønne Petersen wrote:
> No. It's an abomination, and it blocks more novel/interesting language design opportunities.

There have been discussions about the comma operator before. I don't expect that it's going anywhere, even though there are plenty of people who would love to see it gone.

- Jonathan M Davis
April 21, 2012
Le 21/04/2012 14:51, Jonathan M Davis a écrit :
> On Saturday, April 21, 2012 14:25:14 Alex Rønne Petersen wrote:
>> No. It's an abomination, and it blocks more novel/interesting language
>> design opportunities.
>
> There have been discussions about the comma operator before. I don't expect
> that it's going anywhere, even though there are plenty of people who would
> love to see it gone.
>
> - Jonathan M Davis

Add me on the list.
April 21, 2012
Jonathan M Davis:

> There have been discussions about the comma operator before. I don't expect that it's going anywhere,

Maybe there are intermediate solutions between keeping wild commas in D and disallowing them fully. I think most of my bugs caused by commas are similar to the one shown by the OP. This means this is not a common source of bugs:

foo(), bar();

While this is sometimes a trap:

auto x = foo(), bar();

So maybe it's enough to disallow using the last expression of a comma sequence as result of the whole expression? I don't know. I almost never use commas for such purposes. What are the use case for those commas?

Bye,
bearophile
April 21, 2012
On 04/21/2012 06:54 PM, bearophile wrote:
> Jonathan M Davis:
>
>> There have been discussions about the comma operator before. I don't
>> expect that it's going anywhere,
>
> Maybe there are intermediate solutions between keeping wild commas in D
> and disallowing them fully. I think most of my bugs caused by commas are
> similar to the one shown by the OP. This means this is not a common
> source of bugs:
>
> foo(), bar();
>
> While this is sometimes a trap:
>
> auto x = foo(), bar();
>

This is not valid code.

> So maybe it's enough to disallow using the last expression of a comma
> sequence as result of the whole expression?  I don't know. I almost never
> use commas for such purposes. What are the use case for those commas?
>
> Bye,
> bearophile

if(r.front == 'a' && (r.popFront(), r.front) == 'b') { ... }

There are a few similar usages in Phobos. If comma was to be used as a tuple constructor instead, those could be replaced by ( , )[$-1].
April 21, 2012
Timon Gehr , dans le message (digitalmars.D:164810), a écrit :
> 
> There are a few similar usages in Phobos. If comma was to be used as a tuple constructor instead, those could be replaced by ( , )[$-1].

That would be nice.

-- 
Christophe
April 22, 2012
Le 21/04/2012 18:54, bearophile a écrit :
> Jonathan M Davis:
>
>> There have been discussions about the comma operator before. I don't
>> expect that it's going anywhere,
>
> Maybe there are intermediate solutions between keeping wild commas in D
> and disallowing them fully. I think most of my bugs caused by commas are
> similar to the one shown by the OP. This means this is not a common
> source of bugs:
>
> foo(), bar();
>

This is completely redundant with foo(); bar();

I see no benefit from being able to do this.
April 22, 2012
Le 21/04/2012 20:25, Timon Gehr a écrit :
> On 04/21/2012 06:54 PM, bearophile wrote:
>> Jonathan M Davis:
>>
>>> There have been discussions about the comma operator before. I don't
>>> expect that it's going anywhere,
>>
>> Maybe there are intermediate solutions between keeping wild commas in D
>> and disallowing them fully. I think most of my bugs caused by commas are
>> similar to the one shown by the OP. This means this is not a common
>> source of bugs:
>>
>> foo(), bar();
>>
>> While this is sometimes a trap:
>>
>> auto x = foo(), bar();
>>
>
> This is not valid code.
>
>> So maybe it's enough to disallow using the last expression of a comma
>> sequence as result of the whole expression? I don't know. I almost never
>> use commas for such purposes. What are the use case for those commas?
>>
>> Bye,
>> bearophile
>
> if(r.front == 'a' && (r.popFront(), r.front) == 'b') { ... }
>
> There are a few similar usages in Phobos. If comma was to be used as a
> tuple constructor instead, those could be replaced by ( , )[$-1].

That is better.
« First   ‹ Prev
1 2 3