I was working on a project earlier today that stores IP addresses in a database as a uint. For some reason though, some addresses were coming out as 0.0.0.0, despite the fact that if(ip == 0) return; in the only place it actually saves them (which was my first attempted quick fix for the bug).

Turns out the problem was this:

if (arg == typeid(uint)) {
	int e = va_arg!uint(_argptr);
	a = to!string(e);
}


See, I copy/pasted it from the int check, but didn't update the type on the left hand side. So it correctly pulled a uint out of the varargs, but then assigned it to an int, which the compiler accepted silently, so to!string() printed -blah instead of bigblah... which then got truncated by the database, resulting in zero being stored.

I've since changed it to be "auto e = ..." and it all works correctly now.



Anyway I thought I'd share this just because one of the many times bearophile has talked about this as a potentially buggy situation, I was like "bah humbug"... and now I've actually been there!

I still don't think I'm for changing the language though just because of potential annoyances in other places unsigned works (such as array.length) but at least I've actually felt the other side of the argument in real world code now.

On 28-03-2013 21:03, Adam D. Ruppe wrote:
> I was working on a project earlier today that stores IP addresses in a
> database as a uint. For some reason though, some addresses were coming
> out as 0.0.0.0, despite the fact that if(ip == 0) return; in the only
> place it actually saves them (which was my first attempted quick fix for
> the bug).
>
> Turns out the problem was this:
>
> if (arg == typeid(uint)) {
>      int e = va_arg!uint(_argptr);
>      a = to!string(e);
> }
>
>
> See, I copy/pasted it from the int check, but didn't update the type on
> the left hand side. So it correctly pulled a uint out of the varargs,
> but then assigned it to an int, which the compiler accepted silently, so
> to!string() printed -blah instead of bigblah... which then got truncated
> by the database, resulting in zero being stored.
>
> I've since changed it to be "auto e = ..." and it all works correctly now.
>
>
>
> Anyway I thought I'd share this just because one of the many times
> bearophile has talked about this as a potentially buggy situation, I was
> like "bah humbug"... and now I've actually been there!
>
> I still don't think I'm for changing the language though just because of
> potential annoyances in other places unsigned works (such as
> array.length) but at least I've actually felt the other side of the
> argument in real world code now.

This is exactly why many new languages only allow implicit integer conversions where the target type is strictly a >= type with the same sign, i.e. uint -> ulong, short -> int, and so on.

It is indeed very unfortunate that we have these dangerous implicit conversions in D. I would welcome a change to remove them (because it would likely catch real bugs in many cases).



... And, you know, many other changes to the language/compiler over the last couple of releases have broken plenty of my code. I wonder when we'll finally say "this is the D programming language, period". The current situation where some breaking changes are perfectly OK while others are not is kind of ridiculous.

I'm personally in favor of fixing some of the serious issues we have in the language once and for all and then *finally* stabilizing the language. It's ridiculous that we claim the language to be stable (or stabilizing) while we're still actively breaking real code to fix language issues. Fixing language issues is good and we should do it more so we can actually get to a point where we can call D stable. The current situation where some changes get blocked because the reviewer happens to be in a "D is stable" mood is -- sorry, but really -- stupid.

I used to even tell people "we're stabilizing D" when they ask why we don't fix some particular language design issue. I don't anymore, because I realized just how ridiculous this situation has gotten.

Well... end of rant.

-- 
Alex Rønne Petersen
alex@alexrp.com / alex@lycus.org
http://alexrp.com / http://lycus.org

On Thu, 28 Mar 2013 21:03:07 +0100
"Adam D. Ruppe" <destructionator@gmail.com> wrote:
>
> which then got truncated by the database,

While I won't necessarily disagree with the rest, that right there is "the real WTF". A database that silently alters data is unreliable, and therefore fundamentally broken as a database. It should have raised an error instead.

Is this MySQL, by any chance? And if so, are you making sure to use
strict-mode? That might help. From what I can tell, having strict mode
disabled is basically MySQL's "please fuck up half of my data" feature.
Not that I necessarily trust its strict mode to always be right
(which could very well be unfounded pessimism on my part), but it should
at least help.

On Thursday, 28 March 2013 at 21:29:55 UTC, Nick Sabalausky wrote:
> Is this MySQL, by any chance?

Yes, and no on strict mode, I didn't even know it had one!

Adam D. Ruppe:

> if (arg == typeid(uint)) {
> 	int e = va_arg!uint(_argptr);
> 	a = to!string(e);
> }
>
>
> See, I copy/pasted it from the int check, but didn't update the type on the left hand side. So it correctly pulled a uint out of the varargs, but then assigned it to an int, which the compiler accepted silently,

If you remove the implicit uint==>int assignment from D you have to add many cast() in the code. And casts are dangerous, maybe even more than implicit casts. That's why D is the way it is.

Maybe here a cast(signed) is a bit safer.

I didn't write a Bugzilla request to remove the implicit uint==>int assignment. (I think the signed-unsigned comparisons are more dangerous than those signed-unsigned assignments. But maybe too is a problem with no solution).

------------------

Alex Rønne Petersen:

>I'm personally in favor of fixing some of the serious issues we have in
the language once and for all<

That's quite hard to do because the problems are not easy to fix/improve, it takes time and a _lot_ of thinking. You can't quickly fix "shared", memory ownership problems, redesign things to not preclude the future creation of a far more parallel GC, and so on. And even much simpler things like properties need time to be redesigned. Maybe in the D world there's some need for a theoretician, beside Andrei.

But I agree most of the time should now be used facing the larger holes, design problems and missing parts of D, and less on everything else. Because the more time passes, the less easy it becomes to fix/improve those things. It's a shame to have to leave D after all this work just because similar problems get essentially frozen.

Bye,
bearophile

On 03/28/2013 09:03 PM, Adam D. Ruppe wrote:
> I was working on a project earlier today that stores IP addresses in a
> database as a uint. For some reason though, some addresses were coming
> out as 0.0.0.0, despite the fact that if(ip == 0) return; in the only
> place it actually saves them (which was my first attempted quick fix for
> the bug).
>
> Turns out the problem was this:
>
> if (arg == typeid(uint)) {
>      int e = va_arg!uint(_argptr);
>      a = to!string(e);
> }
>
>
> See, I copy/pasted it from the int check, but didn't update the type on
> the left hand side. ...

While I agree that implicit uint <-> int is a bad situation, I think the following practises deserve the larger part of the blame:

- Having too much redundant information in the code.
- Copypasta & edit instead of string mixins / static foreach.

Of course, sometimes there is a significant amount of temptation.

(Also, that code snippet is nowhere near the most convenient line length. Eliminating the temporary completely is a valid option. :o))

On Thu, Mar 28, 2013 at 09:03:07PM +0100, Adam D. Ruppe wrote:
> I was working on a project earlier today that stores IP addresses in a database as a uint. For some reason though, some addresses were coming out as 0.0.0.0, despite the fact that if(ip == 0) return; in the only place it actually saves them (which was my first attempted quick fix for the bug).
> 
> Turns out the problem was this:
> 
> if (arg == typeid(uint)) {
> 	int e = va_arg!uint(_argptr);
> 	a = to!string(e);
> }
> 
> 
> See, I copy/pasted it from the int check, but didn't update the type on the left hand side. So it correctly pulled a uint out of the varargs, but then assigned it to an int, which the compiler accepted silently, so to!string() printed -blah instead of bigblah... which then got truncated by the database, resulting in zero being stored.

IMO, the compiler should insert bounds checks in non-release mode when implicitly converting between signed and unsigned.

Also, I don't like repeating types, precisely for this reason; if that second line had been written:

	auto e = va_arg!uint(_argptr);

then this bug wouldn't have happened. But once you repeat 'uint' twice, there's the risk that you'll forget to update both instances when changing/copying the code. DRY is a good principle to live by when it comes to coding.


> I've since changed it to be "auto e = ..." and it all works correctly now.

Yep! :)


> Anyway I thought I'd share this just because one of the many times bearophile has talked about this as a potentially buggy situation, I was like "bah humbug"... and now I've actually been there!
> 
> I still don't think I'm for changing the language though just because of potential annoyances in other places unsigned works (such as array.length) but at least I've actually felt the other side of the argument in real world code now.

Maybe it's time to introduce cast(signed) or cast(unsigned) to the
language, as bearophile suggests?


T

-- 
Государство делает вид, что платит нам зарплату, а мы делаем вид, что работаем.

On Thursday, March 28, 2013 15:11:02 H. S. Teoh wrote:
> Maybe it's time to introduce cast(signed) or cast(unsigned) to the
> language, as bearophile suggests?

It's not terribly pretty, but you can always do this

auto foo = cast(Unsigned!(typeof(var))var;

or

auto bar = to!(Unsigned!(typeof(var)))(var);

- Jonathan M Davis

On Friday, 29 March 2013 at 01:18:03 UTC, Jonathan M Davis wrote:
> It's not terribly pretty, but you can always do this

We could also do more C++ looking:

unsigned_cast!foo or IFTI or whatever;

On Thursday, 28 March 2013 at 22:12:57 UTC, H. S. Teoh wrote:
> Also, I don't like repeating types, precisely for this reason; if that second line had been written:

Yeah, I usually don't either, but apparently I did here. Murphy's law at work perhaps!