dummzeuch 1427 Posted June 29, 2023 I had some curious bug today with the following code: procedure ReadHeader(out _w, _h, _Depth: Integer); const PGM_Magic_Number = 'P5'; var s: string; begin s := ReadChar + ReadChar; if s <> PGM_Magic_Number then raise ESigError.Create(_('File is not a valid PGM file:') + ' ' + _fn); // more code here end; Which failed the test even though the first call to ReadChar returns 'P' and the second returns '5' (I stepped through it in the debugger) so s should have been 'P5', but the actual value of s was '5P'. Apparently Delphi created code that called the second ReadChar before the first. But why? Maybe because it fills the parameters for the function System._UStrCat3, that it apparently calls to do the concatenation, from right to left? So I changed the code like this: procedure ReadHeader(out _w, _h, _Depth: Integer); const PGM_Magic_Number = 'P5'; var s: string; begin s := ReadChar; s := s + ReadChar; if s <> PGM_Magic_Number then raise ESigError.Create(_('File is not a valid PGM file:') + ' ' + _fn); // more code here end; and it works now. 1 Share this post Link to post
Uwe Raabe 1923 Posted June 29, 2023 Reminds me of this one: Why does a call to GetDIBits fail on Win64? 1 Share this post Link to post
programmerdelphi2k 236 Posted June 29, 2023 3 hours ago, dummzeuch said: I had some curious bug today and using "System.CONCAT( a, b, c, d, e, .... ); string1 := Concat( char1, char2); Share this post Link to post
programmerdelphi2k 236 Posted June 29, 2023 (edited) 4 hours ago, dummzeuch said: But why? When evaluating an expression, is not correct: first evaluate the end (if the result OK then... after evaluating the beginning That is, in the short circuit we must know whether or not we continue evaluating the expression, exactly as Delphi does when evaluating from "left to right" whether a boolean expression continues or not. However, we can only concatenate the values if we know the result of the final part, and then concatenate with the initial part. This happens with "consts" but is not easy see because the breakpoint does not stop/use it! I don't know if I explained it right (using google) NOTE: CONCAT do the same! with 2 functions = rigth to left order... with 3+ functions, the correct order happens: f1 + f2 + f3 s1 := afterThis + mid_func + firstTHIS; s1 := Format('%s %s', [firstTHIS, afterThis]); // force the correct order Edited June 29, 2023 by programmerdelphi2k Share this post Link to post
Anders Melander 1677 Posted June 29, 2023 34 minutes ago, programmerdelphi2k said: When evaluating an expression, is not correct... No. The order is undefined. For example, if you evaluate the expression (A and B and C ) the evaluation order might very well be BCA. Only boolean short circuit evaluation has a defined order. 2 Share this post Link to post
Daniel 417 Posted June 30, 2023 I removed a couple of posts. Please stay on topic. 2 Share this post Link to post
Lars Fosdal 1746 Posted June 30, 2023 I do a lot of string manipulation, but I have not seen this effect yet. Weird. I would definitively have assumed that the concatenation would go left to right on evaluation of the functions. Would this change the outcome? s := ''; s := s + ReadChar + ReadChar; Share this post Link to post
dummzeuch 1427 Posted June 30, 2023 (edited) A simple test case would be this: function ReadChar1: Char: begin Result := 'P'; end; function ReadChar2: Char: begin Result := '5'; end; procedure ReadHeader(out _w, _h, _Depth: Integer); const PGM_Magic_Number = 'P5'; var s: string; begin s := ReadChar1 + ReadChar2; if s <> PGM_Magic_Number then raise ESigError.Create(_('File is not a valid PGM file:') + ' ' + _fn); // more code here end; Edit: I forgot to add for what this is a test case. It's for stepping through with the debugger. Since ReadChar1 and ReadChar2 are two different functions, it's clear in which order they are executed. It's ReadChar2 first and ReadChar1 second. Edited June 30, 2023 by dummzeuch Share this post Link to post
Cristian Peța 96 Posted June 30, 2023 (edited) Probably I do not understand something and you code do not compile for me. This is working as expected for me. It will show "OK". PS. Now I understand: ReadChar2 is called first and will read the first char from file. uses System.SysUtils; function ReadChar1: Char; begin Result := 'P'; end; function ReadChar2: Char; begin Result := '5'; end; procedure ReadHeader(out _w, _h, _Depth: Integer); const PGM_Magic_Number = 'P5'; var s: string; begin s := ReadChar1 + ReadChar2; if s <> PGM_Magic_Number then raise Exception.Create('File is not a valid PGM file:') else Writeln('OK'); end; var _w, _h, _Depth: Integer; begin ReadHeader(_w, _h, _Depth); Readln; end. Edited June 30, 2023 by Cristian Peța Share this post Link to post
Cristian Peța 96 Posted June 30, 2023 But ReadChar2 is called first and will read the first char from file Share this post Link to post
Cristian Peța 96 Posted June 30, 2023 Project1.dpr.20: s := ReadChar1 + ReadChar2; 0008E605 E8BEFFFFFF call $0008e5c8 0008E60A 8BD0 mov edx,eax 0008E60C 8D45EC lea eax,[ebp-$14] 0008E60F E8289CFEFF call $0007823c 0008E614 8B45EC mov eax,[ebp-$14] 0008E617 50 push eax 0008E618 E897FFFFFF call $0008e5b4 0008E61D 8BD0 mov edx,eax 0008E61F 8D45E8 lea eax,[ebp-$18] 0008E622 E8159CFEFF call $0007823c 0008E627 8B55E8 mov edx,[ebp-$18] 0008E62A 8D45FC lea eax,[ebp-$04] 0008E62D 59 pop ecx 0008E62E E8B59DFEFF call $000783e8 First call to $0008E5C8 is ReadChar2. It is with '5'. Project1.dpr.10: begin 0008E5C8 55 push ebp 0008E5C9 8BEC mov ebp,esp 0008E5CB 51 push ecx Project1.dpr.11: Result := '5'; 0008E5CC 66C745FE3500 mov word ptr [ebp-$02],$0035 Project1.dpr.12: end; 0008E5D2 668B45FE mov ax,[ebp-$02] 0008E5D6 59 pop ecx 0008E5D7 5D pop ebp 0008E5D8 C3 ret 0008E5D9 8D4000 lea eax,[eax+$00] Share this post Link to post
Stefan Glienke 1924 Posted June 30, 2023 (edited) Of course, the result will be as expected if you name the two functions differently, then it does not matter what order they are getting called - but when it's twice the same function which returns different results then the call order matters Here is the code to simulate the issue: var callNo: Integer; const PGM_Magic_Number = 'P5'; function ReadChar: Char; begin Inc(callNo); Result := PGM_Magic_Number[callNo]; end; procedure Test; var s: string; begin s := ReadChar + ReadChar; Writeln(s); end; begin Test; end; Edited June 30, 2023 by Stefan Glienke Share this post Link to post
dummzeuch 1427 Posted June 30, 2023 (edited) Sorry for the confusion, I got distracted when I wrote that post. That source code was just for showing that ReadChar2 is called first. I have now added an explanation to that post. Edited June 30, 2023 by dummzeuch Share this post Link to post
Cristian Peța 96 Posted June 30, 2023 (edited) It is because calling convention Right-to-left parameter order? Edited June 30, 2023 by Cristian Peța Share this post Link to post
Stefan Glienke 1924 Posted June 30, 2023 (edited) 16 minutes ago, Cristian Peța said: It is because calling convention Right-to-left parameter order? No, because there are no parameters on ReadChar. No, because even if you mean the string concat method (which is UStrCat3 that will be called here) you will see that if you call ReadChar a third time suddenly order is correct (which then calls UStrCatN) No, because even parameter passing order is undefined behavior in Delphi Edited June 30, 2023 by Stefan Glienke 1 Share this post Link to post
Lars Fosdal 1746 Posted July 3, 2023 Why does your sim code execute in the expected order for me? Share this post Link to post
Uwe Raabe 1923 Posted July 3, 2023 14 minutes ago, Lars Fosdal said: Why does your sim code execute in the expected order for me? Well, as it fails in my case, I guess that is what undefined behavior means. Share this post Link to post
dummzeuch 1427 Posted July 3, 2023 54 minutes ago, Lars Fosdal said: Why does your sim code execute in the expected order for me? No idea. Maybe we are using different Delphi versions (mine is Delphi 2007 in this case) or compiler settings (trying to compare those would open a can of worms). Share this post Link to post
Lars Fosdal 1746 Posted July 3, 2023 Delphi 11.3 28.0.47991.2819 (Update 3 + Patch 1) Quote {$A8,B-,C+,D+,E-,F-,G+,H+,I+,J-,K-,L+,M-,N-,O+,P+,Q-,R-,S-,T-,U-,V+,W-,X+,Y+,Z1} {$MINSTACKSIZE $00004000} {$MAXSTACKSIZE $00100000} {$IMAGEBASE $00400000} {$APPTYPE CONSOLE} {$WARN SYMBOL_DEPRECATED ON} {$WARN SYMBOL_LIBRARY ON} {$WARN SYMBOL_PLATFORM ON} {$WARN SYMBOL_EXPERIMENTAL ON} {$WARN UNIT_LIBRARY ON} {$WARN UNIT_PLATFORM ON} {$WARN UNIT_DEPRECATED ON} {$WARN UNIT_EXPERIMENTAL ON} {$WARN HRESULT_COMPAT ON} {$WARN HIDING_MEMBER ON} {$WARN HIDDEN_VIRTUAL ON} {$WARN GARBAGE ON} {$WARN BOUNDS_ERROR ON} {$WARN ZERO_NIL_COMPAT ON} {$WARN STRING_CONST_TRUNCED ON} {$WARN FOR_LOOP_VAR_VARPAR ON} {$WARN TYPED_CONST_VARPAR ON} {$WARN ASG_TO_TYPED_CONST ON} {$WARN CASE_LABEL_RANGE ON} {$WARN FOR_VARIABLE ON} {$WARN CONSTRUCTING_ABSTRACT ON} {$WARN COMPARISON_FALSE ON} {$WARN COMPARISON_TRUE ON} {$WARN COMPARING_SIGNED_UNSIGNED ON} {$WARN COMBINING_SIGNED_UNSIGNED ON} {$WARN UNSUPPORTED_CONSTRUCT ON} {$WARN FILE_OPEN ON} {$WARN FILE_OPEN_UNITSRC ON} {$WARN BAD_GLOBAL_SYMBOL ON} {$WARN DUPLICATE_CTOR_DTOR ON} {$WARN INVALID_DIRECTIVE ON} {$WARN PACKAGE_NO_LINK ON} {$WARN PACKAGED_THREADVAR ON} {$WARN IMPLICIT_IMPORT ON} {$WARN HPPEMIT_IGNORED ON} {$WARN NO_RETVAL ON} {$WARN USE_BEFORE_DEF ON} {$WARN FOR_LOOP_VAR_UNDEF ON} {$WARN UNIT_NAME_MISMATCH ON} {$WARN NO_CFG_FILE_FOUND ON} {$WARN IMPLICIT_VARIANTS ON} {$WARN UNICODE_TO_LOCALE ON} {$WARN LOCALE_TO_UNICODE ON} {$WARN IMAGEBASE_MULTIPLE ON} {$WARN SUSPICIOUS_TYPECAST ON} {$WARN PRIVATE_PROPACCESSOR ON} {$WARN UNSAFE_TYPE OFF} {$WARN UNSAFE_CODE OFF} {$WARN UNSAFE_CAST OFF} {$WARN OPTION_TRUNCATED ON} {$WARN WIDECHAR_REDUCED ON} {$WARN DUPLICATES_IGNORED ON} {$WARN UNIT_INIT_SEQ ON} {$WARN LOCAL_PINVOKE ON} {$WARN MESSAGE_DIRECTIVE ON} {$WARN TYPEINFO_IMPLICITLY_ADDED ON} {$WARN RLINK_WARNING ON} {$WARN IMPLICIT_STRING_CAST ON} {$WARN IMPLICIT_STRING_CAST_LOSS ON} {$WARN EXPLICIT_STRING_CAST OFF} {$WARN EXPLICIT_STRING_CAST_LOSS OFF} {$WARN CVT_WCHAR_TO_ACHAR ON} {$WARN CVT_NARROWING_STRING_LOST ON} {$WARN CVT_ACHAR_TO_WCHAR ON} {$WARN CVT_WIDENING_STRING_LOST ON} {$WARN NON_PORTABLE_TYPECAST ON} {$WARN XML_WHITESPACE_NOT_ALLOWED ON} {$WARN XML_UNKNOWN_ENTITY ON} {$WARN XML_INVALID_NAME_START ON} {$WARN XML_INVALID_NAME ON} {$WARN XML_EXPECTED_CHARACTER ON} {$WARN XML_CREF_NO_RESOLVE ON} {$WARN XML_NO_PARM ON} {$WARN XML_NO_MATCHING_PARM ON} {$WARN IMMUTABLE_STRINGS OFF} Off-topic: Ctrl+O + Ctrl+O behaves weirdly, IMO. It inserts at top of file instead of at cursor, and it doesn't respect existing options like {$APPTYPE CONSOLE} Share this post Link to post
Attila Kovacs 619 Posted July 3, 2023 28 minutes ago, dummzeuch said: No idea. Maybe we are using different Delphi versions (mine is Delphi 2007 in this case) or compiler settings (trying to compare those would open a can of worms). There is no UStrCat3 in D2007, and it works perfectly. Meanwhile, newer versions do have it and are failing. Now you are telling us that yours is failing with D2007, and Lars' is working with D11? Share this post Link to post
dummzeuch 1427 Posted July 3, 2023 6 minutes ago, Attila Kovacs said: There is no UStrCat3 in D2007, and it works perfectly. Meanwhile, newer versions do have it and are failing. Now you are telling us that yours is failing with D2007, and Lars' is working with D11? Now that you mention it: There was some Unicode stuff involved somewhere else in the unit (type casts from AnsiChar to Char), so it can't have been Delphi 2007 ... Must have been Delphi 10.2 then. (Sorry, I have been trying to get somebody competent at Deutsche Telekom on the phone for the last 2 hours. This is getting on my nerves and wreaking havoc with my concentration.) 1 Share this post Link to post
Uwe Raabe 1923 Posted July 3, 2023 As expected, I indeed get different outcomes with Win32 and Win64 targets. (Delphi 11.3 Patch 1) Share this post Link to post