in reply to Re: Warnings on unused variables?
in thread Warnings on unused variables?

Mhm, thanks for the explanation, though I would not have considered cases 2, 3 or 6 false positives (those are the kinds of things I would want to catch, honestly), and would not have considered 1, 4 and 5 single-use (#1 uses $foo++, #4 in theory will see $lock checked at least once inside do_stuff_that_needs_the_lock(), and #5 passes $tree to new()).

Anyway, I took Grinder's point that use warnings::unused could be left commented out for commits, and went with that. It's Worked For Me™ so far.

Thanks, all.

Replies are listed 'Best First'.
Re^3: Warnings on unused variables?
by ikegami (Patriarch) on Sep 27, 2008 at 18:27 UTC

    I would not have considered cases 2, 3 or 6 false positives

    Why not? A warning would be issued for complete, accurate, functioning code. That's the very definition of a false positive.

    #2 ignores one of the function's return values. There are other ways of doing it, but there's nothing wrong with that way.

    #3 implements creates an empty file.

    #6 is used in creating inside out objects.

    #1 uses $foo++

    No. As far as Perl is concerned, '$foo++' might as well be 'print "Hello World"' when the program is compiled. $foo is only seen long after the warning has been issued, when the eval is executed.

    #4 in theory will see $lock checked at least once inside do_stuff_that_needs_the_lock()

    No. It's not passed to do_stuff_that_needs_the_lock (which would be using it twice).

    #5 passes $tree to new()

    Yeah, but what about $sentinel?

    #4 and #5 are both examples of Resource Acquisition Is Initialization objects where the functionality exists solely in the constructor and the (implicitly called) destructor.

      Mhm, either you and I have different definitions of 'accurate', or we have different conceptions about what constitutes a useful warning. I want warnings on code that, especially if handed to someone who didn't write it, is more likely than equivalent code that doesn't generate the warning to become the seed of a future bug. To me, a warning says, "Okay, well, yes, you can technically do it that way, but it's likely to be better in the long run if you do something else."

      Specifics follow the jump.

        I just tested that #1 in fact does use $foo++, and warnings::unused handles it properly

        You forgot about tied variables. $foo++ can be tied to code that prints out "Hello World" as a side effect.

        I guess the short version of this is, I don't see any situation where using a declare-once-and-forget variable is ever clearer or substantially more efficient than an alternative, so I don't see any of these situations as warranting exceptions from warnings, even if they are in common use.

        You haven't programmed much with resource acquisition/release systems then. C has lots of problems with that, Windows COM has lots of problems with that and Perls reference counting also has these problems (although many of them have been fixed in all instances). With Reference Counting as the instance of resource acquisition and release, you have exactly the problem that the points of acquisition (increase of refcount) and release (decrement of refcount) are not in the same place of the code and hence, quite unpleasantly, not every acquisition is matched with a release, because it's hard to see that they match up because they do not happen in the same place. Which is why a "execute this code at scope exit" functionality is wanted.

        #3 indicates a position where a filehandle was used for an open and never used again, not even to check return value on a close.

        close will never fail, cause all we want to do is create the file.

        It adds all of a dozen characters

        But that's a dozen of characters of unnecessary code. For just one line of code! I'd have useless code all over the place. Code that can do nothing but introduce errors, since it doesn't serve any function.

        and leaves the warnings valid for any time I had opened multiple files, and then accidentally used one of the file descriptors twice in a row instead of using the one I intended.

        No. Since you believe in closing everything explicitly, the warning won't help you there.

        # $fh1 $fh2 # ---- ---- open(my $fh1, '>', $file1) or die; # 1 open(my $fh2, '>', $file2) or die; # 1 for (@data) { # print $fh1 "$_\n"; # 1 print $fh1 "$_\n"; # Oops # 1 } # close($fh1); # 1 close($fh2); # 1 # ---- ---- # 4 2 -> no warnings

        Furthermore, autodie is a great module for testing errors on close, and it doesn't reference the file handle explicitly.

        I'm not sure why putting your locks in your constructor

        No, the constructor obtains the lock. The object is the lock.

        I would not personally use constructor methods named in such a way that they can be easily mistaken for procedures

        You've never used threads? And constructors are routinely called names other than new.

        I'm not sure why [RAII] is substantially better [...] explicitly locking the object until it passes out of scope.

        To properly release the lock when an exception occurs or when return is used more than once.

        but I'd argue that this is exactly why I want a warning on code that looks like that.

        I'd rather avoid doing away with exceptions, I'd rather my code doesn't throw spurious warnings, and I'd rather not have to us no warnings; all over the place. That's why it's better checked by a linter or until turned on explicitly (meaning -v and use warnings; wouldn't enable it).

        You've hidden functionality inside a destructor-only event that is going to trigger many lines of code away from when you prepped it. I want a warning if someone does that.

        You say deallocation of resources on block exit worthy of a warning? Every single variable in Perl does that!

        In to the other class of examples, I just tested that #1 in fact does use $foo++, and warnings::unused handles it properly:

        That's odd.
        Does eval '$'.'foo++' issue a warning for $foo? (It shouldn't)
        Does eval 'print q{$foo++}' issue a warning for $foo? (It should)
        Does eval '$cond ? $foo : $bar' issue a warning for $foo and $bar? (It shouldn't)