From e24449bcfd24e482e286b294830bcc76044c4a82 Mon Sep 17 00:00:00 2001
From: florian <florian@freepascal.org>
Date: Sun, 20 Jan 2019 12:55:20 +0000
Subject: [PATCH] * if sub nodes of a commutative node contain conditionally
 executed nodes, these sub nodes might not be swapped   as this might result
 in some nodes not being executed, like temp. create nodes with init. code,
 see e.g. issue #34653, resolves #34653

git-svn-id: trunk@40934 -
---
 .gitattributes          |  1 +
 compiler/ncgutil.pas    |  8 +++++++-
 compiler/nutils.pas     | 20 ++++++++++++++++++++
 tests/webtbs/tw34653.pp | 22 ++++++++++++++++++++++
 4 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 tests/webtbs/tw34653.pp

diff --git a/.gitattributes b/.gitattributes
index 41cb6bfca5..02fcdff997 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -16470,6 +16470,7 @@ tests/webtbs/tw3456.pp svneol=native#text/plain
 tests/webtbs/tw3457.pp svneol=native#text/plain
 tests/webtbs/tw3460.pp svneol=native#text/plain
 tests/webtbs/tw34605.pp svneol=native#text/plain
+tests/webtbs/tw34653.pp svneol=native#text/pascal
 tests/webtbs/tw3467.pp svneol=native#text/plain
 tests/webtbs/tw3470.pp svneol=native#text/plain
 tests/webtbs/tw3474.pp svneol=native#text/plain
diff --git a/compiler/ncgutil.pas b/compiler/ncgutil.pas
index 67aa3ce6e4..6bc0e9c841 100644
--- a/compiler/ncgutil.pas
+++ b/compiler/ncgutil.pas
@@ -221,7 +221,13 @@ implementation
                  (fcl>0)) or
                 (((fcr=fcl) or
                   (fcr=0)) and
-                 (ncr>ncl)) then
+                 (ncr>ncl)) and
+                { if one tree contains nodes being conditionally executated, we cannot swap the trees
+                  as the other tree might depend on all nodes being executed, this applies for example
+                  for temp. create nodes with init part, they must be executed else things break, see
+                  issue #34653
+                }
+                 not(has_conditional_nodes(p.right)) then
                p.swapleftright
            end;
       end;
diff --git a/compiler/nutils.pas b/compiler/nutils.pas
index 274be0653d..d807c8bd46 100644
--- a/compiler/nutils.pas
+++ b/compiler/nutils.pas
@@ -127,6 +127,9 @@ interface
     { returns true, if the tree given might have side effects }
     function might_have_sideeffects(n : tnode;const flags : tmhs_flags = []) : boolean;
 
+    { returns true, if n contains nodes which might be conditionally executed }
+    function has_conditional_nodes(n : tnode) : boolean;
+
     { count the number of nodes in the node tree,
       rough estimation how large the tree "node" is }
     function node_count(node : tnode) : dword;
@@ -1378,6 +1381,23 @@ implementation
         result:=foreachnodestatic(n,@check_for_sideeffect,@flags);
       end;
 
+
+    function check_for_conditional_nodes(var n: tnode; arg: pointer): foreachnoderesult;
+      begin
+        result:=fen_false;
+        { this check is not complete yet, but sufficent to cover the current use case: swapping
+          of trees in expressions }
+        if (n.nodetype in [ifn,whilerepeatn,forn,tryexceptn]) or
+          ((n.nodetype in [orn,andn]) and is_boolean(n.resultdef) and doshortbooleval(n)) then
+          result:=fen_norecurse_true;
+      end;
+
+
+    function has_conditional_nodes(n : tnode) : boolean;
+      begin
+        result:=foreachnodestatic(n,@check_for_conditional_nodes,nil);
+      end;
+
     var
       nodecount : dword;
 
diff --git a/tests/webtbs/tw34653.pp b/tests/webtbs/tw34653.pp
new file mode 100644
index 0000000000..260dfd709f
--- /dev/null
+++ b/tests/webtbs/tw34653.pp
@@ -0,0 +1,22 @@
+{ %OPT=-O2 }
+var d: LongInt;
+begin
+    WriteLn;
+
+    d := 828;
+    if ((d mod 2) = 0) xor ((d < 0) and ((d mod 3) = 0)) then
+      WriteLn('YES')
+    else
+      begin
+        WriteLn('NO');
+        halt(1);
+      end;
+
+    if ((d mod 2) = 0) xor ((d < 0) and ((d mod 3) = 0)) then
+        WriteLn('YES')
+    else
+      begin
+        WriteLn('NO');
+        halt(1);
+      end;
+end.