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 @@ public void RewriteForLoops(List<AbsynStatement> stmts)
  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();