Comment #0 by bearophile_hugs — 2010-03-05T10:31:12Z
The following small program:
void main() {
int x = 5;
x + 3;
}
Currently produces the compile error:
Error: + has no effect in expression (x + 3)
because that's often a small bug in the program.
Equally, a pure function has no side effects, so its only purpose is to return a value. So the compiler can issue a warning or error if its return value is not used. This can help avoid some small bugs.
This predecessor() function computes the predecessor of its input argument, its result is always nonnegative:
pure int predecessor(int x) {
if (x <= 0)
throw new Exception("");
return x - 1;
}
void main() {
predecessor(5); // warning or error
}
In theory a pure function can be used just for the exceptions it throws, but I think this is a quite uncommon usage.
A "pure nothrow" function can't even have this uncommon purpose, so this can produce a compile error:
pure nothrow int predecessor(int x) {
return x - 1;
}
void main() {
predecessor(5); // error
}
--------------
In GCC there is a function attribute named "warn_unused_result":
http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html
The warn_unused_result attribute causes a warning to be emitted if a caller of the function with this attribute does not use its return value. This is useful for functions where not checking the result is either a security problem or always a bug, such as realloc.
Similar attribute can be named like @do_use_result in D2.
Comment #1 by bearophile_hugs — 2011-02-28T04:21:28Z
See also bug 5464 , that contains and expands just the second part of this enhancement request (because putting two different requests in a bugzilla issue is bad).
Comment #2 by bearophile_hugs — 2011-03-27T07:29:53Z
If the druntime function to allocate a new array is a pure function, then this enhancement request catches a bug like this too:
{
new int[10];
}
Comment #3 by bearophile_hugs — 2011-03-27T08:39:01Z
KennyTM~ reminds that this is error message is only for "strongly pure" functions.
Comment #4 by bearophile_hugs — 2011-03-27T13:21:22Z
Comment #6 by bearophile_hugs — 2011-06-25T23:14:11Z
(In reply to comment #5)
> I added this as a warning because it's fairly onerous now that we have implicit
> purity deduction.
Thank you very much Walter. DMD 2.054 is becoming an interesting release.
In few months the warning will allow us to see _how much_ onerous a similar error is.
Comment #7 by bearophile_hugs — 2011-06-27T12:51:57Z
I change this to WONTFIX because practice has already shown this change is not currently practical. More thinking is needed.
Comment #8 by hoganmeier — 2012-01-05T14:38:57Z
*** Issue 7235 has been marked as a duplicate of this issue. ***
Comment #9 by bearophile_hugs — 2013-03-02T05:36:15Z
*** Issue 9632 has been marked as a duplicate of this issue. ***
Comment #12 by bearophile_hugs — 2014-03-01T15:20:21Z
(In reply to comment #11)
> This pull request adds warnings for unused non-void returns of calls to
> strictly pure function calls.
Is adding a cast(void) before the function call able to disable this warning? If this is false then perhaps it's worth adding this sub-feature and then I suggest a shorter and more useful warning message like:
Warning: Function call to pure function foo() discards return value, prepend a cast(void) if intentional.
> For this to work Phobos had to altered in handful
> of places were non-void returns were not captured.
Some user code will need similar changes...
Comment #13 by per.nordlow — 2014-03-01T15:28:11Z
(In reply to comment #12)
> (In reply to comment #11)
>
> > This pull request adds warnings for unused non-void returns of calls to
> > strictly pure function calls.
>
> Is adding a cast(void) before the function call able to disable this warning?
Yes, adding cast(void) in front of expression disables warning.
> If this is false then perhaps it's worth adding this sub-feature and then I
> suggest a shorter and more useful warning message like:
>
> Warning: Function call to pure function foo() discards return value, prepend a
> cast(void) if intentional.
This is a very good idea. I'll implement this.
It still think it would be useful to also show the context of the called function. What do you think?
> > For this to work Phobos had to altered in handful
> > of places were non-void returns were not captured.
>
> Some user code will need similar changes...
I believe its worth the stretch because it can help us capture potential bugs. It caught two for me.
Comment #14 by bearophile_hugs — 2014-03-01T15:43:03Z
(In reply to comment #13)
> It still think it would be useful to also show the context of the called
> function. What do you think?
What's the context of the called function?
Do you mean a message like this?
Warning: Function call to pure function foo(x) discards int return value, prepend a cast(void) if intentional.
Comment #15 by per.nordlow — 2014-03-01T15:47:07Z
(In reply to comment #14)
> (In reply to comment #13)
>
> > It still think it would be useful to also show the context of the called
> > function. What do you think?
>
> What's the context of the called function?
I mean the place were it's defined.
Here's my proposal:
t.d(30,17): Warning: Call to strictly pure function f() discards return value, prepend a cast(void) if intentional
t.d(18,22): function defined here
Comment #16 by bearophile_hugs — 2014-03-01T16:16:25Z
(In reply to comment #15)
> Here's my proposal:
>
> t.d(30,17): Warning: Call to strictly pure function f() discards return value,
> prepend a cast(void) if intentional
> t.d(18,22): function defined here
In general if a programmer is using an IDE it's easy to click on the function name at the call point, and jump to the function definition. So the second line is not necessary for programmers that use an IDE.
How is dmd acting in other situations for other errors/warnings? I think for functions the second line where the function is defines is usually not given.
So perhaps a single error message like this is enough:
test.d(30,17): Warning: discarded return value of pure function 'test.foo', prepend a cast(void) if intentional
Comment #17 by per.nordlow — 2014-03-01T16:25:25Z
> In general if a programmer is using an IDE it's easy to click on the function
> name at the call point, and jump to the function definition. So the second line
> is not necessary for programmers that use an IDE.
I don't think there are any IDEs that support semantic resolution of overloaded and/or templated functions especially not when they are called through UFCS.
Maybe we should add an extra switch for extra context information...
> How is dmd acting in other situations for other errors/warnings? I think for
> functions the second line where the function is defines is usually not given.
>
> So perhaps a single error message like this is enough:
>
> test.d(30,17): Warning: discarded return value of pure function 'test.foo',
> prepend a cast(void) if intentional
D has both strict and weak purity. This issue is about strict purity. Shouldn't this be mentioned in the warning?
Also: Does anyone know how to add the scope (in this case 'test') qualifier to the function name?
Comment #18 by bearophile_hugs — 2014-03-01T16:35:31Z
(In reply to comment #17)
> I don't think there are any IDEs that support semantic resolution of overloaded
> and/or templated functions especially not when they are called through UFCS.
Then add the context too, and let's see what Walter/Kenji think about it.
>Shouldn't this be mentioned in the warning?
OK.
> Does anyone know how to add the scope (in this case 'test') qualifier to
the function name?
My suggestion is to search for the "cannot call impure function" string in the compiler source code and copy how it's done there.
Comment #19 by per.nordlow — 2014-03-01T16:53:11Z
> My suggestion is to search for the "cannot call impure function" string in the
> compiler source code and copy how it's done there.
Do you mean toPrettyChars()? I don't understand its behaviour. What does
test.f!int.f mean?:
Warning: Call to strictly pure function test.f!int.f discards return value, prepend a cast(void) if intentional
given the code
@safe pure nothrow T f(T)(T x)
{
return x*x;
}
void main(string args[])
{
int x = 3;
f(x);
}
What do you think?
Comment #20 by bearophile_hugs — 2014-03-01T16:59:27Z
(In reply to comment #19)
> What does test.f!int.f mean?
> ...
> @safe pure nothrow T f(T)(T x)
> {
> return x*x;
> }
> void main(string args[])
> {
> int x = 3;
> f(x);
> }
f here is not a function, it's a function template.
This code:
@safe pure nothrow T f(T)(T x) {
return x*x;
}
Is the same as:
template f(T) {
@safe pure nothrow T f(T x) {
return x*x;
}
}
So when you call f inside the test module with an int you are actually calling test.f!int.f
For a templated function the second f is redundant in the error message.
Comment #21 by per.nordlow — 2014-03-01T17:01:58Z
(In reply to comment #20)
> (In reply to comment #19)
>
> > What does test.f!int.f mean?
> > ...
> > @safe pure nothrow T f(T)(T x)
> > {
> > return x*x;
> > }
> > void main(string args[])
> > {
> > int x = 3;
> > f(x);
> > }
>
> f here is not a function, it's a function template.
>
> This code:
>
> @safe pure nothrow T f(T)(T x) {
> return x*x;
> }
>
> Is the same as:
>
> template f(T) {
> @safe pure nothrow T f(T x) {
> return x*x;
> }
> }
>
> So when you call f inside the test module with an int you are actually calling
> test.f!int.f
>
> For a templated function the second f is redundant in the error message.
Ahh, get it.
I guess I still need to figure out how to print test.f instead right?
Comment #22 by github-bugzilla — 2014-03-09T12:10:49Z
Comment #23 by bearophile_hugs — 2014-03-09T12:47:48Z
Perhaps I am missing something, but I am not seeing a warning with this code:
int foo(int x) pure nothrow {
return x - 1;
}
void main() {
foo(5);
}
Comment #24 by bearophile_hugs — 2014-03-09T12:50:34Z
(In reply to comment #23)
> Perhaps I am missing something, but I am not seeing a warning with this code:
>
>
> int foo(int x) pure nothrow {
> return x - 1;
> }
> void main() {
> foo(5);
> }
Ignore my comment please, the commit apparently was just for Phobos. Sorry.
Comment #25 by github-bugzilla — 2014-03-12T22:14:09Z
> It's not a good idea to Inhibit more DMD warnings introducing spurious
> variables that could generate new "variable not used" warnings:
If you're interested I could try looking into adding support in DMD for emitting such warnings. If so could someone guide to a place in the DMD sources where this check should be added?
> A better fix is to use the cast(void) everywhere.
Ok, I'll change this today and then do a new pull request for Phobos.
Comment #28 by bearophile_hugs — 2014-03-18T06:53:41Z
(In reply to comment #27)
> If you're interested I could try looking into adding support in DMD for
> emitting such warnings.
This is an entirely new topic, so it must be discussed in a different Bugzilla entry.
I am interested in such warning, but by design D doesn't like warnings a lot, so if you create such warning, be prepared to see your pull request rejected (but it could be a good try anyway).
> If so could someone guide to a place in the DMD sources
> where this check should be added?
Unfortunately you have to ask this to people that know the DMD compiler more than me.
> > A better fix is to use the cast(void) everywhere.
>
> Ok, I'll change this today and then do a new pull request for Phobos.
Good.
Comment #29 by github-bugzilla — 2014-03-18T20:03:41Z
There's a corresponding pull request for dmd that hasn't been merged yet because Phobos still generates warnings (assumeSafeAppend).
I don't understand why I don't get these warnings when I do
make unittest
on Ubuntu box.
I'll fix these on Phobos and try again.
Comment #32 by bearophile_hugs — 2014-03-19T02:39:47Z
(In reply to comment #31)
> There's a corresponding pull request for dmd that hasn't been merged yet
Then the issue is reopened until the dmd pull request is merged.
Comment #33 by github-bugzilla — 2014-03-20T12:06:10Z
Comment #35 by bearophile_hugs — 2014-03-21T12:37:55Z
Now this code:
int foo(int x) pure nothrow {
return x - 1;
}
void main() {
foo(5);
}
Gives:
test.d(5,8): Warning: Call to strictly pure function test.foo discards return value of type int, prepend a cast(void) if intentional
Issue fixed.
Comment #36 by github-bugzilla — 2014-03-22T15:16:12Z