Use while for infinite loops

[sharwell]

I would like to propose the implementation and inclusion of a new StyleCop diagnostic.

Category: Readability
Rule: Use while for infinite loops
Description: Infinite loops can be expressed as either while (true) or for (;;). The former should always be used.

Background: Some older compilers, especially older non-optimizing C++ compilers or script compilers, produced sub-optimal code for a while (true) - e.g. they would emit code to evaluate the expression true followed by a conditional branch instruction. These same compilers would recognize the empty increment and condition blocks of a for (;;) statement, and emit an optimized unconditional branch instruction without any other evaluation. As a result, many highly-experienced developers who previously worked with these compilers have a habit of writing for (;;) in code. Developers coming from most other backgrounds tend to write while (true) instead.

Rationale: The C# compiler does not impose technical or performance limitations that restrict our ability to make a decision on this matter. As such, I believe that the decision should be made based on what the majority of C# developers would find easiest to read. I also believe this is while (true).

Analysis: There are several ways this could be implemented which impact the overall scope.

1. This could be limited to locating specific cases of for(;;).
2. This could be limited to locating cases of for(;{expression}?;).
3. This could be limited to locating cases of any for statement which does not have an increment expression (the initializer, if any, could be placed before the while statement when converting to it). This introduces an additional challenge for a code fix because changing an initializer expression to a variable declaration statement changes the scope of a variable, and would need to handle any conflicts which arose.

[vweijsters]

For readability I would prefer option 3, but then the scope of the diagnostic should be widened (or we should have an additional diagnostic for for loops without increment expression).

For this diagnostic it would probably best if we stick to option 1 and I do think we should create another diagnostic for when not to use for loops (especially since a lot of people will assume that an increment expression is present, upon reading the for keyword, without truly checking the code)

Can we also extended this to include do { ... } while (true);, so that we truly get a single notation for infinite loops?

[Przemyslaw-W]

I also think this particular rule should target only scenario 1. So that infinite loops are consistent across whole codebase.

Regarding the potential new diagnostic targeted at scenarios 2 and 3. IMHO it should be another rule. I assume it would be something like "Don't use for loop with empty increment (decrement) expression". Possible code fixes should not only offer rewrite to use better loop. But also allow to insert missing increment (or decrement) expression - when for loop has filled initializer.

[AArnott]

If we do any, I'd suggest option 1 as well. However as I haven't seen anyone try to write the for (;;) syntax in C# for an infinite loop, it doesn't strike me as a particularly useful rule. But if others have seen it, then sure, I don't object to it.

[sharwell]

I found two instances of it in Roslyn.

[jackwhelpton]

I've always written an infinite loop as for (;;) just because it's slightly more concise than the while (true) equivalent; I hadn't realised this was unusual. For completeness, it's worth noting that StyleCopAnalyzers raise a violation of SA1002 when you do this, which the original version never did.

I've raised #1489 for discussion of that.

[GregReddick]

Us old C hacks are used to for(;;), but I think while(true) is easier to understand. I think 3 is the right choice. The code fix can change

for (int foo = 1;;)
{
}

to

int foo = 1;
while (true)
{
}

but if foo would cause a conflict, then wrap the whole thing in another set of braces. Presumably, since this is ugly, the programmer would then change the variable name. So the code fix would do this only in cases of conflict. This:

            for (int foo = 1; ;)
            {
                if (foo == 1)
                {
                    break;
                }
            }

            for (int foo = 1; foo < 10; foo++)
            {
                Console.WriteLine(foo);
            }

would turn into this:

            {
                int foo = 1;
                while (true)
                {
                    if (foo == 1)
                    {
                        break;
                    }
                }
            }

            for (int foo = 1; foo < 10; foo++)
            {
                Console.WriteLine(foo);
            }

The other alternative is to have the code fix refuse to operate if it would cause a problem.