Are situations like this, that are potential signs of bugs/mistakes, worth reporting with warnings?


int foo() {
    // Unconditional 'break' within a loop:
    foreach (immutable _; 0 .. 10) {
        //...
        break;
    }

    // Unconditional 'return' within a loop:
    foreach (immutable _; 0 .. 10) {
        //...
        return 0;
    }

    assert(0);
}


Bye,
bearophile

On Tuesday, 13 August 2013 at 14:56:32 UTC, bearophile wrote:
> Are situations like this, that are potential signs of bugs/mistakes, worth reporting with warnings?
>
>
> int foo() {
>     // Unconditional 'break' within a loop:
>     foreach (immutable _; 0 .. 10) {
>         //...
>         break;
>     }
>
>     // Unconditional 'return' within a loop:
>     foreach (immutable _; 0 .. 10) {
>         //...
>         return 0;
>     }
>
>     assert(0);
> }
>
>
> Bye,
> bearophile

Arguably, D has the "unreachable code" warning, so an unconditional break would short circuit the incrementation code.

So I think there is no need for a "new" warning for this, but to make it detect that a certain amount of code is not reachable.

On 08/13/2013 04:56 PM, bearophile wrote:
> Are situations like this, that are potential signs of bugs/mistakes,
> worth reporting with warnings?
>
>
> int foo() {
>      // Unconditional 'break' within a loop:
>      foreach (immutable _; 0 .. 10) {
>          //...
>          break;
>      }
>
>      // Unconditional 'return' within a loop:
>      foreach (immutable _; 0 .. 10) {
>          //...
>          return 0;
>      }
>
>      assert(0);
> }
>
>
> Bye,
> bearophile

Well, the following is the most generic way to get the first element of some iterable:

foreach(x;s) return x;

monarch_dodra:

> Arguably, D has the "unreachable code" warning, so an unconditional break would short circuit the incrementation code.
>
> So I think there is no need for a "new" warning for this, but to make it detect that a certain amount of code is not reachable.

The D compiler gives a warning if you add code after those return and break. So the missing warning is useful when those return and break are at the end of the loop body.

That assert(0) is needed because D doesn't deduce that return is always executed.

Bye,
bearophile

Timon Gehr:

> Well, the following is the most generic way to get the first element of some iterable:
>
> foreach(x;s) return x;

I think I have not used that idiom even with opApply, but it's acceptable code. And perhaps someone will find a use case even for the unconditional loop break.

So I think this little idea-proposal is already closed :-)

Bye,
bearophile

If you get bored:
1. Run "dscanner --ast" on some source code
2. Create an XPath expression to find the code you don't like
3. Create a tool that checks the expression against the AST
4. Enjoy your low-budget static code analysis tool.

(This idea shamelessly stolen from PMD)

https://github.com/Hackerpilot/Dscanner/
http://pmd.sourceforge.net/pmd-5.0.5/xpathruletutorial.html

On Tuesday, August 13, 2013 19:58:28 Brian Schott wrote:
> If you get bored:

With so much stuff to do for D, how could you ever get bored. ;)

- Jonathan M Davis


P.S. Keep up the good work.