* 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

@@ -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

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;

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;
 

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.