warn_unused_results

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

warn_unused_results

Peter Eisentraut-6
Forgetting to assign the return value of list APIs such as lappend() is
a perennial favorite.  The compiler can help point out such mistakes.
GCC has an attribute warn_unused_results.  Also C++ has standardized
this under the name "nodiscard", and C has a proposal to do the same
[0].  In my patch I call the symbol pg_nodiscard, so that perhaps in a
distant future one only has to do s/pg_nodiscard/nodiscard/ or something
similar.  Also, the name is short enough that it doesn't mess up the
formatting of function declarations too much.

I have added pg_nodiscard decorations to all the list functions where I
found it sensible, as well as repalloc() for good measure, since
realloc() is usually mentioned as an example where this function
attribute is useful.

I have found two places in the existing code where this creates
warnings.  Both places are correct as is, but make assumptions about the
internals of the list APIs and it seemed better just to fix the warning
than to write a treatise about why it's correct as is.


[0]: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2051.pdf

--
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

0001-Add-pg_nodiscard-function-declaration-specifier.patch (1K) Download Attachment
0002-Add-pg_nodiscard-decorations-to-some-functions.patch (8K) Download Attachment
0003-Silence-some-pg_nodiscard-warnings.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: warn_unused_results

Tom Lane-2
Peter Eisentraut <[hidden email]> writes:
> Forgetting to assign the return value of list APIs such as lappend() is
> a perennial favorite.  The compiler can help point out such mistakes.
> GCC has an attribute warn_unused_results.  Also C++ has standardized
> this under the name "nodiscard", and C has a proposal to do the same
> [0].  In my patch I call the symbol pg_nodiscard, so that perhaps in a
> distant future one only has to do s/pg_nodiscard/nodiscard/ or something
> similar.  Also, the name is short enough that it doesn't mess up the
> formatting of function declarations too much.

+1 in principle (I've not read the patch in detail); but I wonder what
pgindent does with these added keywords.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: warn_unused_results

Peter Eisentraut-6
On 2020-10-17 17:58, Tom Lane wrote:

> Peter Eisentraut <[hidden email]> writes:
>> Forgetting to assign the return value of list APIs such as lappend() is
>> a perennial favorite.  The compiler can help point out such mistakes.
>> GCC has an attribute warn_unused_results.  Also C++ has standardized
>> this under the name "nodiscard", and C has a proposal to do the same
>> [0].  In my patch I call the symbol pg_nodiscard, so that perhaps in a
>> distant future one only has to do s/pg_nodiscard/nodiscard/ or something
>> similar.  Also, the name is short enough that it doesn't mess up the
>> formatting of function declarations too much.
>
> +1 in principle (I've not read the patch in detail); but I wonder what
> pgindent does with these added keywords.

pgindent doesn't seem to want to change anything about the patched files
as I had sent them.

--
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services