From e4af0fc1e989534551aa5df34d8fced260bf2787 Mon Sep 17 00:00:00 2001
From: florian <florian@freepascal.org>
Date: Sat, 16 Nov 2024 17:27:16 +0100
Subject: [PATCH]   * don't mess with boolean and/or operations in mixed
 short/full evaluated expressions, resolves #41012

---
 compiler/optcse.pas     | 13 ++++++++-----
 tests/webtbs/tw41012.pp | 15 +++++++++++++++
 2 files changed, 23 insertions(+), 5 deletions(-)
 create mode 100644 tests/webtbs/tw41012.pp

diff --git a/compiler/optcse.pas b/compiler/optcse.pas
index 5e5eccac51..e249305d32 100644
--- a/compiler/optcse.pas
+++ b/compiler/optcse.pas
@@ -356,9 +356,9 @@ unit optcse;
                    (is_set(n.resultdef))
                    ) then
                   while (n.nodetype=tbinarynode(n).left.nodetype) and
-                    { if node (1) is fully boolean evaluated and node (2) not, we cannot do the swap as this might result in B being evaluated always,
-                      the other way round is no problem, C is still evaluated only if needed }
-                    (not(is_boolean(n.resultdef)) or not(n.nodetype in [andn,orn]) or doshortbooleval(n) or not(doshortbooleval(tbinarynode(n).left))) and
+                    { if not both or none of the nodes is short/full boolean evaluated, we cannot do this optimization as it might change
+                      semantics, there are border cases where this might not apply, but so far it is not worth the effort to check }
+                    (not(is_boolean(n.resultdef)) or not(n.nodetype in [andn,orn]) or (doshortbooleval(n)=doshortbooleval(tbinarynode(n).left))) and
                         { the resulttypes of the operands we'll swap must be equal,
                           required in case of a 32x32->64 multiplication, then we
                           cannot swap out one of the 32 bit operands for a 64 bit one
@@ -374,10 +374,13 @@ unit optcse;
                           foreachnodestatic(pm_postprocess,tbinarynode(tbinarynode(n).left).right,@searchsubdomain,@csedomain);
                           if csedomain then
                             begin
-                              { move the full boolean evaluation of (2) to (1), if it was there (so it again applies to A and
+                              {
+                                move the full boolean evaluation of (2) to (1), if it was there (so it again applies to A and
                                 what follows) }
+                              { this cannot happen anymore, see above, so the condition is not needed anymore
+
                               if not(doshortbooleval(tbinarynode(n).left)) and
-                                 doshortbooleval(n) then
+                                 doshortbooleval(n) then }
                                 begin
                                   n.localswitches:=n.localswitches+(tbinarynode(n).left.localswitches*[cs_full_boolean_eval]);
                                   exclude(tbinarynode(n).left.localswitches,cs_full_boolean_eval);
diff --git a/tests/webtbs/tw41012.pp b/tests/webtbs/tw41012.pp
new file mode 100644
index 0000000000..15e2b17af1
--- /dev/null
+++ b/tests/webtbs/tw41012.pp
@@ -0,0 +1,15 @@
+program test;
+{$mode objfpc}
+
+function GetFalse: Boolean;
+begin
+  Result := False;
+end;
+
+var
+  obj: TInterfacedObject;
+  i: Integer;
+
+begin
+  if GetFalse and (i >= 0) and (i < obj.RefCount) then;
+end.
\ No newline at end of file