Feature: Canonicalize do-whiles with empty bodies

Feature requested in #1287.

src/Decompiler/Structure/ForLoopRewriter.cs

@@ -23,17 +23,16 @@
 using Reko.Core.Expressions;
 using Reko.Core.Operators;
 using Reko.Core.Types;
-using System;
 using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
 
 namespace Reko.Structure
 {
+    /// <summary>
+    /// Detects and rewrites loop constructs.
+    /// </summary>
     public class ForLoopRewriter
     {
-        private Procedure proc;
+        private readonly Procedure proc;
         private readonly ExpressionValueComparer cmp;
 
         public ForLoopRewriter(Procedure proc)
@@ -125,8 +124,8 @@ public void RewriteForLoops(List<AbsynStatement> stmts)
                     // Pattern 1
                     return new AbsynWhile(doCond, doWhile.Body);
                 }
-                if (!(ifStm.Condition is BinaryExpression ifCondBin) ||
-                    !(doWhile.Condition is BinaryExpression doCondBin))
+                if (ifStm.Condition is not BinaryExpression ifCondBin ||
+                    doWhile.Condition is not BinaryExpression doCondBin)
                     return null;
 
                 if (ifCondBin.Operator.Type == OperatorType.Ne && 
@@ -159,8 +158,8 @@ ifCondBin.Right is Constant ifConst && ifConst.IsZero &&
             List<AbsynStatement> container, 
             int i)
         {
-            if (!(cond is BinaryExpression cmp) ||
-                !(cmp.Operator is ConditionalOperator))
+            if (cond is not BinaryExpression cmp ||
+                cmp.Operator is not ConditionalOperator)
                 return null;
 
             var (loopVariable, update) = IdentifyLoopVariable(loopBody, cmp);

src/Decompiler/Structure/StructureAnalysis.cs

@@ -1061,8 +1061,17 @@ public bool ReduceCyclic(Region n)
                         // DoWhile!
                         var exp = s == succs[0]
                             ? n.Expression!.Invert()
-                            : n.Expression;
-                        loopStm = new AbsynDoWhile(s.Statements, exp!);
+                            : n.Expression!;
+                        // #1287: do-while's with empty bodies are identical to whiles,
+                        // and whiles are more idiomatic.
+                        if (s.Statements.Count == 0)
+                        {
+                            loopStm = new AbsynWhile(exp, s.Statements);
+                        }
+                        else
+                        {
+                            loopStm = new AbsynDoWhile(s.Statements, exp);
+                        }
                         n.Type = RegionType.Linear;
                     }
                     n.Statements = new List<AbsynStatement> { loopStm };

src/Decompiler/Structure/TailReturnRemover.cs

@@ -30,7 +30,7 @@
 namespace Reko.Structure
 {
     /// <summary>
-    /// Removes reduntant 'return' statements in procedures that return
+    /// Removes redundant 'return' statements in procedures that return
     /// void. These return statements will always be in tail position.
     /// </summary>
     public class TailReturnRemover : IAbsynVisitor<bool>

src/UnitTests/Decompiler/Structure/AbsynCodeEmitter.cs

@@ -50,7 +50,7 @@ public void Declare(Identifier id, Expression initializer=null)
             stmts.Add(decl);
         }
 
-        public void DoWhile(Action<AbsynCodeEmitter> bodyGen, BinaryExpression cond)
+        public void DoWhile(Action<AbsynCodeEmitter> bodyGen, Expression cond)
         {
             var bodyStmts = new List<AbsynStatement>();
             var m = new AbsynCodeEmitter(bodyStmts);

src/UnitTests/Decompiler/Structure/SimpleStructureTests.cs

@@ -27,6 +27,8 @@
 using System;
 using System.Diagnostics;
 using System.IO;
+using Reko.Core.Expressions;
+using Reko.Core.Types;
 
 namespace Reko.UnitTests.Decompiler.Structure
 {
@@ -371,5 +373,29 @@ public void StrReg00568()
         {
             RunTest("Fragments/regressions/r00568.asm", "Structure/StrReg00568.txt");
         }
+
+        [Test(Description = "A polling loop is idiomatically expressed as a while(poll) ; ")]
+        [Category(Categories.UnitTests)]
+        public void StrPollingLoop()
+        {
+            var pm = new ProgramBuilder();
+            pm.Add(nameof(StrPollingLoop), m =>
+            {
+                var pollFn = Identifier.Create(new TemporaryStorage("poll", 1, PrimitiveType.Ptr32));
+                m.Label("lupe");
+                m.BranchIf(m.Fn(pollFn), "lupe");
+                m.Return();
+            });
+            var sExp =
+@"define StrPollingLoop
+{
+	while (poll())
+		;
+}
+===
+";
+            RunTest(sExp, pm.Program);
+        }
+
     }
 }

src/tests/Structure/StrReg00013.exp

@@ -229,9 +229,8 @@ void fn0C00_0000(selector ds)
 		if (fn0C00_0267(0x800<16>))
 			goto l0C00_0251;
 	}
-	do
+	while (Mem50[0800:81B0:word16] != 0<16> && Mem50[0800:5404:word16] < 0x20<16>)
 		;
-	while (Mem50[0800:81B0:word16] != 0<16> && Mem50[0800:5404:word16] < 0x20<16>);
 	fn0C00_0283(0x5450<16>, 0x800<16>);
 	Mem72[0800:5404:word16] = 0<16>;
 	fn0C00_0259();
@@ -270,9 +269,8 @@ void fn0C00_0000(selector ds)
 			ax_131 = SLICE(eax_136, word16, 0) *s 0xF000<16>;
 		}
 		fn0C00_0265();
-		do
+		while (Mem248[ds_123:0x81B0<16>:word16] != 0<16> && Mem248[ds_123:0x5404<16>:word16] < 0x46<16>)
 			;
-		while (Mem248[ds_123:0x81B0<16>:word16] != 0<16> && Mem248[ds_123:0x5404<16>:word16] < 0x46<16>);
 		fn0C00_028F(0x53E7<16>, ds_123);
 		fn0C00_028B();
 		fn0C00_027F();