http://d.puremagic.com/issues/show_bug.cgi?id=7989

           Summary: isInputRange and isForwardRange declare unused
                    variables
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Phobos
        AssignedTo: nobody@puremagic.com
        ReportedBy: smjg@iname.com
            Blocks: 3960


--- Comment #0 from Stewart Gordon <smjg@iname.com> 2012-04-26 07:02:21 PDT ---
http://dlang.org/function.html
"It is an error to declare a local variable that is never referred to. Dead
variables, like anachronistic dead code, are just a source of confusion for
maintenance programmers."

However, as Timon Gehr has pointed out on digitalmars.D.learn, std.range.isInputRange breaks this rule, and I've discovered that isForwardRange does too.

template isInputRange(R)
{
    enum bool isInputRange = is(typeof(
    {
        R r = void;       // can define a range object
        if (r.empty) {}   // can test for empty
        r.popFront();     // can invoke popFront()
        auto h = r.front; // can get the front of the range
    }));
}

template isForwardRange(R)
{
    enum bool isForwardRange = isInputRange!R && is(typeof(
    {
        R r1 = void;
        R r2 = r1.save; // can call "save" against a range object
    }));
}

h and r2 are unused variables.  These templates only work at the moment because of issue 3960.  Fixing that issue would at the moment cause these templates to always evaluate to false, since the declarations of h and r2 are illegal because the variables are never referred to.

Possible solutions:

-        auto h = r.front; // can get the front of the range +        cast(void) r.front; // can get the front of the range

-        R r2 = r1.save; // can call "save" against a range object
+        static assert(is(typeof(r1.save) : R));

If issue 3960 is dealt with by removing the statement from the spec instead, there is still some support for a warning about it, so it would be still worth dealing with this in order to avoid such a warning.

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

http://d.puremagic.com/issues/show_bug.cgi?id=7989


Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy@yahoo.com


--- Comment #1 from Steven Schveighoffer <schveiguy@yahoo.com> 2012-04-26 07:17:50 PDT ---
(In reply to comment #0)

> -        auto h = r.front; // can get the front of the range +        cast(void) r.front; // can get the front of the range

front should return a value, returning void is not acceptable, but this would pass your code.

I think this might work better:

static assert(is(typeof(r.front) != void));

It may also be required that front can be used to initialize a variable (I think there may be some cases where a non-void value cannot be used this way, but I'm not sure).  How do you test for this, and then 'use' the variable later when you have no idea how it can be used?  I suppose you could pass it to a function template?

This kind of requirement is going to be problematic, possibly in a lot of places.  It might make sense to define some more helper enums for this, like isNonVoidInitializer or something like that.

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

http://d.puremagic.com/issues/show_bug.cgi?id=7989



--- Comment #2 from Stewart Gordon <smjg@iname.com> 2012-04-26 07:30:49 PDT ---
(In reply to comment #1)
> It may also be required that front can be used to initialize a variable (I think there may be some cases where a non-void value cannot be used this way, but I'm not sure).  How do you test for this, and then 'use' the variable later when you have no idea how it can be used?  I suppose you could pass it to a function template?

Maybe

auto h = r.front;
static assert(is(typeof(h)));

(I've assumed here that using a variable in a static assert or IsExpression passes as referencing it.)

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

http://d.puremagic.com/issues/show_bug.cgi?id=7989



--- Comment #3 from Stewart Gordon <smjg@iname.com> 2012-04-26 07:58:16 PDT ---
(In reply to comment #1)
> I suppose you could pass it to a function template?

I've just figured that would work as well.  Define

void nop(T...)(T t) {}

then this could be used to
(a) require that something is of a non-void type
(b) require that something is implicitly convertible to a given type
(c) reference an otherwise unreferenced variable, thus suppress the unused
variable error/warning

so the lines I suggested would become

nop(r.front);
nop!(R)(r1.save);

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

http://d.puremagic.com/issues/show_bug.cgi?id=7989


Jonathan M Davis <jmdavisProg@gmx.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jmdavisProg@gmx.com


--- Comment #4 from Jonathan M Davis <jmdavisProg@gmx.com> 2012-04-26 10:44:34 PDT ---
I'd say that this is a very argument for why the compiler _shouldn't_ complain about unused variables at all.

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

http://d.puremagic.com/issues/show_bug.cgi?id=7989



--- Comment #5 from Stewart Gordon <smjg@iname.com> 2012-04-26 10:52:39 PDT ---
(In reply to comment #4)
> I'd say that this is a very argument for why the compiler _shouldn't_ complain about unused variables at all.

What are you referring to as "this" exactly?

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

http://d.puremagic.com/issues/show_bug.cgi?id=7989



--- Comment #6 from Jonathan M Davis <jmdavisProg@gmx.com> 2012-04-26 11:07:51 PDT ---
> What are you referring to as "this" exactly?

This bug report. The fact that isInputRange and isForwardRange rely on declaring variables which aren't used being legal. It would be really annoying for unused local variables to be illegal when dealing with template constraint stuff like isInputRange and isForwardRange. Code would have to be needlessly contorted to deal with that fact, and you wouldn't ever get a good error about why the result of the template was false, because it would be part of a template constraint.

IHMO, the very issue that this bug report brings up highlights a good reason why unused local variables should continue to be ignored by the compiler.

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

http://d.puremagic.com/issues/show_bug.cgi?id=7989



--- Comment #7 from Stewart Gordon <smjg@iname.com> 2012-04-26 11:48:45 PDT ---
This discussion is relevant to both this and issue 3960, so I'm continuing it on digitalmars.D.learn under the existing "Docs: Section on local variables" thread.

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