www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 4694] New: Unused last assignment warning

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4694

           Summary: Unused last assignment warning
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Keywords: diagnostic
          Severity: enhancement
          Priority: P2
         Component: DMD
        AssignedTo: nobody puremagic.com
        ReportedBy: bearophile_hugs eml.cc



This enhancement request is about a warning that helps avoid some kinds of
bugs.

(This is related but distinct from the enhancement request of bug 3960
This has nothing to do with uninitialized variables, it's kind of the opposite
problem).

The "Unused last assignment warning" means that you assign something to a
variable, and then you do nothing with it.

The compiler is often able to remove the dead code, but the presence of this
situation may denote low quality code, or code where the programmer has
forgotten to use the variable. In both cases it's useful to find such
situations.


void main() {
    int x;
    x = 10; // warning here
}


Here 'x' is not an unused variable, because the code does something with it, so
there is no warning for unused variable (bug 3960 ), but this code deserves a
warning anyway because the last value assigned to x is not unused.

It is better to not show this warning if the variable address is assigned to a
pointer or similar things.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 20 2010
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4694




Answers to a comment by Jonathan M Davis:

Okay. Giving a warning for an unused variable makes sense. However, giving a
warning for setting it somewhere that is not where it is declared doesn't make
sense. Sure, in this case, it's dumb of the programmer to have done that, but

1. The compiler should be able to optimize out such a simple case.

2. More complex cases much harder to detect, and it's not all that hard for you
to _want_ to disconnect the declaration of the variable and initializing it -
likely because of scoping issues or conditional blocks. The compiler is only

cases will likely get optimized away. Cases too complex to optimize away are
going to be much harder for the compiler to detect, and if you start going that
far, you're getting to the point where you just about should have done what
Java did and force initialization of variables rather than default initialize
them.

Not initializing a variable yourself, and the setting it later with a value
that you could have set it to at its declaration is not a coding error. It may
not be best practice, but it's not going to result in an error. So, I don't
think that it makes any sense to make it a warning.

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


 However, giving a warning for setting it somewhere that is not where it is
 declared doesn't make sense.


This is not the purpose of this warning.

The purpose of this warning is to spot situations where you assign something to
a variable and then you forget to use such contents. It's kind of the opposite
of the unused variable.



 1. The compiler should be able to optimize out such a simple case.


The compiler is often able to remove dead code, but the purpose of this warning
is to detect a point where code is written badly (no need for an operation) or
it's a possible sign of a spot where the programmer has forgotten to use a
variable.



 2. More complex cases much harder to detect,


This is true, but this is just a warning, it's not a change in the language, so
even if the compiler is not able to spot the most complex situations, it's OK
still. A partial help against bugs is better than no help.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 20 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4694


Jonathan M Davis <jmdavisProg gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jmdavisProg gmail.com



15:08:49 PDT ---
I'm okay with this as long as it gives no false positives. Unlike with the
variable initialization issue, where you might have to initialize something
when you don't need to, a false positive in this case would result in having to
remove an assignment that you needed. Presumably, if this were implemented, it
would not be done in a manner that that would ever happen. So, given that, it
seems like a fine request to me. I don't think that it's something that would
help me much, but it wouldn't hurt. The main question, however, is whether
Walter would ever even considering it, since one, he hates warnings, and two,
this involves code flow analysis which he generally avoids.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 20 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4694


Stewart Gordon <smjg iname.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |smjg iname.com





 The main question, however, is whether
 Walter would ever even considering it, since one, he hates warnings, and two,
 this involves code flow analysis which he generally avoids.


Does picking out the most trivial cases constitute control flow analysis?

Cases where the variable isn't in a loop contained within the variable's scope
and is not referenced at all between the assignment and the end of the scope
certainly ought to be spotted.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 10 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4694




See also  Issue 2197

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Apr 24 2012
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4694




I have found another important use case for this diagnostic enhancement
request.

In my code three times or more I have put a not easy to find bug. Maybe the
original code was using nested dynamic arrays, or class instances:


void main() {
    auto data = [[10], [20], [30]];
    foreach (f; data)
        f[0]++;
}


And for some reason I replace the nested arrays with a struct or tuple:

import std.typecons: tuple;
void main() {
    alias tuple T;
    auto data = [T(10), T(20), T(30)];
    foreach (f; data)
        f[0]++;
}


The bug is that changes in f are not altering the array data. The correct code
is:


import std.typecons: tuple;
void main() {
    alias tuple T;
    auto data = [T(10), T(20), T(30)];
    foreach (ref f; data)
        f[0]++;
}







struct F {
    public int x;
}
public class Test {
    public static void Main() {
        F[] data = {new F { x = 10 }, new F { x = 20 }, new F { x = 30 }};
        foreach (F f in data)
            f.x++;        
    }
}




test_s.cs(8,13): error CS1654: Cannot assign to members of `f' because it is a
`foreach iteration variable'
Compilation failed: 1 error(s), 0 warnings



A way to help avoid this bug in D is to add the unused last assignment warning:

import std.typecons: tuple;
void main() {
    alias tuple T;
    auto data = [T(10), T(20), T(30)];
    foreach (f; data)
        f[0]++;
}


A warning similar to:

temp.d(6): Warning: last assignment to f[0] is unused

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 28 2012