[Bug tree-optimization/18487] Warnings for pure and const functions that are not actually pure or const

["federico.kircheis at gmail dot com" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=18487

--- Comment #26 from Federico Kircheis <federico.kircheis at gmail dot com> ---
As multiple people commented this Ticket, I do not know to who the least
message is sent, but I would like to give again my opinion on it, as I would
really like to use those attributes in non-toy projects.

> This seems like a bad idea

I think there are valid use-cases for those warnings.


> and is impossible in general

Let me quote myself:

> ... a warning that even only works for trivial case is much better than nothing, because at least I know I can safely use the attribute for some functions as a contract to the caller, and have it checked.

There are now two possible outcomes if a compiler emits a warning.

1)
I look at the definition, and *gasp*, the compiler is actually right.
The function was pure before, but the last changes made it impure.
Either I did not realize it, or I forgot to change the function declaration.
Thank you GCC for making me aware of the issue, I'll fix it.

2) 
I look at the definition an think that GCC is wrong.
I know better, and the function is pure.
I can either try to simplify the function in such a way that GCC does not
complain anymore (which might be a good idea), or I can use a pragma to ignore
this one warning (and comment why it's ignored), or remove the attribute
altogether, as GCC might call the function multiple times if it thinks it's
impure (see example at the end).
In the first approach, I can still benefit from warnings if the function
changes again.
In the second case I cant but at least, I can still grep in the entire codebase
and check periodically which warnings have been disabled locally, just like I
do for other warnings.
In the third case yes, I would probably report a bug with a minimal example.
This (hopefully), would improve GCC analysis capabilities.


> The whole point of the attributes is to tell the compiler things are pure/const in cases it can't already prove.

That does not mean that it is not useful to let it do the check, *especially if
it can prove that the attribute is used incorrectly*, but even if it can't
prove anything.
And also see the example at the end why this is not completely true.

> It can already prove a lot, and doesn't need help in most of the simple examples being given (in other bugs). 


But programmers (at least for the most use-cases I've seen) needs that type of
support.
I would like to know if a function has side effects.
It's great if the compiler can see it automatically, but when reading and
writing code, especially code not written by me or maintained by multiple
authors, we might want to restrict the functionality of some functions.

For side-effect free functions, the attributes const and pure are great, but
using them is more harmful, because if used wrongly it introduces UB, thus

1) they do not really document if a function is pure, as there is no tooling
checking if the statement is true
2) they introduce bugs that no-one can explain (see at the end).

Thus a comment "this function is pure", is by contrast much better, as it does
not introduce UB, but we all know that those kind of commends do not age well.
Thus at the end, they get ignored because not trustworthy, and one need always
to look at the implementation.

> You are basically going to warn in the cases the compiler can't prove it [...]

And for many use-cases it is fine.




Also the second example I gave:

----
// bar.hpp
[[gnu::const]] int get_value();

// bar.cpp
int get_value(){static int i = 0; return ++i;}


// foo.cpp
int foo(){
    int i = get_value();
    int j = get_value();
    return i+j;
}
----

The compiler will still optimize the call to get_value, (unless it is able to
see the definition of get_value and see that there are side effects).

Thus, if the function is marked pure, the compiler

* will not call it a second time if it does not see the implementation of
`get_value`
* will call it a second time if it sees the implementation of `get_value` and
notices it is not pure.

This is one of those bugs that no-one can explain, as simply moving code
(making a function, for example, inline, or move it to another file), or
changing optimization level, changes the behavior of the program.


Thus, given main.cpp

----
[[gnu::const]] int foo();

// foo.cpp
int main(){
    int i = foo();
    int j = foo();
    return i+j;
}
----


how many times is GCC going to call foo?

If GCC thinks that the function is pure, then only once.
If it thinks it is not pure, twice.

I have no idea what GCC thinks, because there are no diagnostics for it!
And look, it does not even matter if foo is pure or not, it matters if GCC
thinks if it is pure or not. 

I can similarly tell GCC to inline functions, but if GCC doesn't at least it
will tell me he didn't.(warning: 'always_inline' function might not be
inlinable [-Wattributes])



We can of course say "those attributes are only for those people that really
know better", but as the compiler is already checking if a function is pure or
not (as described, if it sees the implementation it does not elide the second
call), some information is already available.
I fail to see how not making this information visible to the end user who is
doing an error, and making thus those attributes available for more
programmers, is a bad thing.

I'm unable to judge how difficult it is (I suppose a lot as the bug report is
of 2004), and this might be the reason GCC will never be able to output the
information, but please do not dismiss it because "it works as designed", as me
(and suppose other reporters) believe it could be more useful than it currently
is, without invalidating it's original use-case.