Explorar o código

* 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
(cherry picked from commit e24449bcfd24e482e286b294830bcc76044c4a82)

# Conflicts:
# .gitattributes

florian %!s(int64=6) %!d(string=hai) anos
pai
achega
c69fde258c
Modificáronse 3 ficheiros con 49 adicións e 1 borrados
  1. 7 1
      compiler/ncgutil.pas
  2. 20 0
      compiler/nutils.pas
  3. 22 0
      tests/webtbs/tw34653.pp

+ 7 - 1
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;

+ 20 - 0
compiler/nutils.pas

@@ -119,6 +119,9 @@ interface
     { returns true, if the tree given might have side effects }
     function might_have_sideeffects(n : tnode) : 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;
@@ -1377,6 +1380,23 @@ implementation
         result:=foreachnodestatic(n,@check_for_sideeffect,nil);
       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;
 

+ 22 - 0
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.