True or False or False = False!
True or False or False = False!
(True or False or False) = True.
function TPSDConnectionMonitor.IsOnline: Boolean;
var
cState: TPSDConnectionState;
begin
Result := (Not OfflineStoreEnabled) or (Not Enabled) or IsOnline(cState);
end;
See the picture.
FOfflineStoreEnabled = False
FEnabled = True
IsOnline(cState) does an and between two states and returns False
The expression
(Not OfflineStoreEnabled) or (Not Enabled) or IsOnline(cState)
is (True) or (False) or False
and Result is False! #WTF
Add extra paranthesis to the expression
( (Not OfflineStoreEnabled) or (Not Enabled) or IsOnline(cState))
is ((True) or (False) or False)
and Result is True as expected!
Can someone make sense of this?
#XE5
That's something you should check in the CPU view, the debugger could be telling you lies (ie. you have an heisenbug)
ReplyDeleteIs this only the debugger showing wrong or did the compiler actually generate wrong code? (try by putting out the values to OutputDebugString or so)
ReplyDeleteWell, it's real- It executed as False until I added the parenthesis, so it's not a heisenbug.
ReplyDeleteI'll have a look at the code generated.
Doh! It was some sort of Heisenbug after all.
ReplyDeleteA removal of parenthesis and clean and build does not replicate the issue.
This is not the first time I've seen it, though - but it was the first time I recorded a screenshot.
Recently Christoph Hillefeld had a similiar problem:
ReplyDeletex := True and True and True;
But after x was False... But i didnt remember his Solution^^
As far as I remember this kind of compilation tricks are the result of the shortcut boolean evaluation, without the use of explicit parenthesis it wil become an error/bug. You have two options, complete boolean evaluation or explicit parenthesis, I recomend the second one, never fails (at least to my personal experience).
ReplyDeleteWell, it looks like compiler developers didnt hear about unit tests.
ReplyDeleteTotally agree with Heinz Toskano here, I always put an enclosing set of parentheses on a compound logic evaluation like that. There are always issues with the boolean logic not quite working otherwise, I'm sure I remember a white paper about it, can't remember the why or wherefore of it though.
ReplyDeleteAFAIK it's to do with the compiler settings around boolean evaluation, but I can't honestly remember.
Heinz Toskano - Short circuit Boolean evaluation will stop execution
ReplyDelete- at first False for and statements
- at first True for or statement
Docs state: "In the {$B-} state, the compiler generates code for short-circuit Boolean expression evaluation, which means that evaluation stops as soon as the result of the entire expression becomes evident in left to right order of evaluation."
Parenthesis should not affect the evaluation as sub expressions also have the short circuit eval applied , although I do agree that explicit use of parenthesis is a good thing for readability many times.
In my example above, the first (not false) should still have returned true.
function TPSDConnectionMonitor.IsOnline : Boolean;
ReplyDeletevar
cState: TPSDConnectionState;
begin
try
Result := (Not OfflineStoreEnabled) or (Not Enabled) or IsOnline(cState);
exception
on E : Exception do
Showmessage(E.Message);
end;
end;
??? :) Exception in IsOnline method ???
By default result is false.
ReplyDeleteIsOnline is first in the order and ...
Dobrin Petkov - cState is an out argument, and that version of IsOnline is exception safe, dealing only with copied values, not pointers or objects. Besides evaluation is LEFT to right, and no exception is raised. OfflineStoreEnabled is the first, and if (not OfflineStoreEnabled) is true, that should be the result.
ReplyDeleteLars, can you make repeatable example, with project options et cetera.
ReplyDeleteVladimir Srednikh - Unfortunately, the bug disappeared after a build. I did repeatedly see it happen before the rebuild, but I am unable to reproduce it. Next time, I'll make sure that I capture the assembly code as well.
ReplyDelete