Add annotations to functions

[SiegeLord]

E.g. __attribute__(warn_unused_result) on the various bool-returning functions like al_init. Maybe even __attribute__(nonnull) for some arguments (although this might be dangerous). This could be implemented either via more ALLEGRO_FUNC macros, or perhaps as separate attribute macros (I think I prefer the former).

See https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes and https://msdn.microsoft.com/en-us/library/hh916383.aspx.

[fatcerberus]

I have mixed feelings on this. On the one hand it would prevent a lot of bugs caused by not checking for failures (I've run into a few crashes due to this), but on the other hand there are some functions where you don't usually need to check the result, either because the chance of failure is low or there's not much you can do even if it does (e.g. malloc failures), and the warnings would be annoying in that case.

[elias-pschernig]

I'm not happy about a warning when not checking the return value of al_init either. I'd say if we add this, then only to functions where we are absolutely sure not checking the return value means a programming error - for example ignoring the result of al_map_rgba or al_lock_bitmap.

[ghost]

Locking can be useful without the ALLEGRO_LOCKED_REGION... just saying.

[SiegeLord]

Yeah, al_lock_bitmap shouldn't warn. I'd rather err on the side of not putting an attribute rather than introducing annoying warnings.

[beoran]

I think the deprecated attribute is probably the most useful one if we are going to switch to only having a current branch in Allegro 5.2. For the rest, some extra warnings could be nice when used judiciously, but they are less essential.

Kind Regards,

B.

[katastic]

What about having them on in the debug version of Allegro? Then telling newbies to always use the debug version.

[bambams]

You can probably suppress the warnings by adding (void) before the call that you're ignoring. I think this would be a good change. Many users that might not want it (i.e., because they intentionally ignore results) also probably ignore warnings. :tongue: It would force users to think about whether or not there's a reason they're ignoring the result (and often, they don't realize there is a result). The perfect program checks every return value. If it can't fail then it shouldn't have a return value. If it can fail then it will. Eventually. At the worst time, and only when you don't check for it. :tongue:

[elias-pschernig]

I don't think (void) is a good idea. That like exceptions in Java... basically every Java code has 100s of lines like:

try { fun(); } except (Exception e) {System.out.println(e);}

Java IDEs just automatically put it around every single line of code now. The original idea of forcing you to catch exceptions was good in theory, but it's just not how things work in practice. As a result Java programs crash more often than any other software now, but hey, at least they print the exception to stdout o_O

[bambams]

Users littering their code with (void) to suppress warnings is no worse than ignoring the return values. If anybody is so ignorant as to add (void) without considering why it suppresses the warning and what it's all for then they deserve what they get. And that's the same guy that would have ignored all of the return values and posted his code to the forum asking why his program crashes. It'll still crash, and we'll have to help him in the same way. But we can create a (void) wiki article explaining why using (void) is wrong because reasons (in particular, because they created that thread to ask us what's wrong) and refuse to help further until they listen.

It's not the catching exceptions in Java that is the problem. It's the required markup explicitly stating what might be thrown, and how this isn't automatically inherited by callers. I'm rusty, but IIRC you are required to say if a method can throw, and I think maybe even what it can throw. Perhaps using static analysis it would be possible to ensure that all exceptions were eventually caught instead, but in practice you'd just end up with the same try { ... } catch { print ... } construct at the entry point.

At that point, you might as well build that in. Which is essentially what Python has done, from the looks of things. It's generally bad to force inconvenient behavior because lazy people are lazy. However, C is already like the try ... catch pass pattern. It already ignores the "exceptions" because the exceptions are just integers. That's even worse. That would be like every statement in Java being implicitly wrapped in try ... catch pass unless explicitly wrapped in try ... catch DONT pass.

This won't affect run-time. It will only pollute the compile-time output with warnings. Which any competent programmer will try to squash correctly, and any other programmer will ignore as usual. Even if they suppress them with (void) nothing is lost. They'll get exactly same experience they do now with a bit more verbosity in the code (harmless verbosity, like using this where it's implicit, which I personally do anyway).

When they (void), whether they know it or not, they'll be declaring, I don't care about this value. I think that it will just help us to communicate the importance of checking return values. You're essentially making work for the programmer either way. They might as well do the right thing and save everybody, including themselves, trouble.