Unreachable code and runtime error 201 on SimpleMsgPack

[cpicanco]

WARNING
Docs: http://www.freepascal.org/docs-html/user/userse65.html

Warning: unreachable code
    You specified a construct which will never be executed. Example:
     while false do 
       begin 
       {.. code ...} 
       end; 

ERROR
Docs: http://www.freepascal.org/docs-html/user/userap4.html

201 Range check error
    If you compiled your program with range checking on, then you can get this error in the following cases:

        An array was accessed with an index outside its declared range.
        Trying to assign a value to a variable outside its range (for instance an enumerated type).

Source: https://github.com/cpicanco/msgpack-delphi/blob/05ee5b40af1a4bd00b1c71b68c458066ac6b2b3b/SimpleMsgPack.pas#L2109

Code: Pascal  [Select][+][-]

1. procedure TSimpleMsgPack.setAsString(pvValue: string);
2. begin
3.   FDataType := mptString;
4.   if SizeOf(Char) = 2 then // <<<<<<<<<<<<<<<< warning
5.   begin
6.     SetLength(FValue, length(pvValue) shl 1);
7.     Move(PChar(pvValue)^, FValue[0], Length(FValue));
8.   end else
9.   begin
10.     SetLength(FValue, length(pvValue));
11.     Move(PChar(pvValue)^, FValue[0], Length(FValue)); // <<<<<<<<< error Runtime (201)
12.   end;
13. end;

private var FValue: TBytes =array

I am getting the error in Ubuntu 14.04 but only sometimes in Debian 8. How to properly resolve the error?

[Fungus]

SizeOf(Char) will always be 1 in FPC, and 2 in newer Delphi's since FPC uses UTF8 in memory and Delphi uses Unicode / WideString. The code in the the conditional block "if SizeOf(Char) = 2 then" will be unreachable in FPC. TBytes is AFAIK declared as "TBytes = array[0..0] of Byte", this means that it is an array of one byte. Using the array in stack will yield a range check error - the TBytes array is normally used as a buffer in memory ( GetMem(MyPBytes, 1024); MyPBytes^[123]:= 123; ). You can disable range check errors with the conditional directive "{$RANGECHECKS OFF}" - not sure if you can disable the unreachable code warnings.

[cpicanco]

Thanks for the reply Fungus! I will test and return the results.

ps.: I am not interested in disable anything.  I am interested in properly resolving the problem!

[cpicanco]

Just to note: TBytes is defined as array of byte. Length is variable...

As far as I am understanding, the way to go is conditional compiling. The SizeOf function is kind of being misused... correct?

[Thaddy]

Well. That code is as you wrote it simply not for UTF8, but for either AnsiString or UnicodeString (which is 16bits)
If you declare all strings as string the code works for Ansi and mostly UTF8, if you declare the units as {$mode delphiunicode} then it works for 16 bits unicodestrings. In the latter case string becomes unicodestring.
There seem to be no huge dependencies on Delphi specific code otherwise.

Remark: I don't understand why you do not use generecs.collections  in other modes than UNICODE. In Freepascal the generics.collections  work in all string modes.
I ran some tests is plain FPC and all code works as you advertised. For Lazarus I suggest it is better to simply adapt the demo when UTF8 is needed and leave the rest of the sourcecode alone.

TBytes is indeed declared in sysutils as a dynamic array of byte.

[cpicanco]

Hi Thaddy, thank you for the reply. So, to make the code fully UTF8 compatible it should be {$IFDEF FPC} FVar : UTF8string?

NOTE : It's not my code. I am improving its FPC compatibility, as a volunteer, by heart.

[Thaddy]

Hi Theddy, thank you for the reply. So, to make the code fully UTF8 compatible it should be {$IFDEF FPC} FVar : UTF8string?

Plz NOT because in FPC plain the code is already working as it should. The problem is Lazarus, NOT FPC. Thats why I suggest to simply adapt just the demo.

[Thaddy]

Maybe better to overload some of the functions that take a string as parameter:

Code: Pascal  [Select][+][-]

1. procedure TSimpleMsgPack.setAsString(pvValue: UTF8String);overload;
2. procedure TSimpleMsgPack.setAsString(pvValue: AnsiString);overload;
3. procedure TSimpleMsgPack.setAsString(pvValue: UnicodeString);overload;

etc.

[cpicanco]

Maybe better to overload some of the functions that take a string as parameter:

Code: Pascal  [Select][+][-]

1. procedure TSimpleMsgPack.setAsString(pvValue: UTF8String);
2. procedure TSimpleMsgPack.setAsString(pvValue: AnsiString);
3. procedure TSimpleMsgPack.setAsString(pvValue: UnicodeString);

etc.

Ok! I will test this and return the results, seems pretty! I will link to this discussion if the work turns to a Pull Request. Thank you

[cpicanco]

Anyway, the warning should be resolved by compiler directives.... But this would require some research...

[Thaddy]

BTW if you test the code with a console app in plain Freepascal you will see that it already works as you expected.
It is just that lazarus default to UTF8. This should be resolved automatically in the future. Try adding LazUTF8 to the uses clause, maybe just that is enough. Changes string to codepage CP_UTF8 automatically from FPC 3.0.0

[cpicanco]

BTW if you test the code with a console app in plain Freepascal you will see that it already works as you expected.
It is just that lazarus default to UTF8. This should be resolved automatically in the future.

Ok, I got it. Lazarus defaults to UTF8, I will consider this point. But I really need to compile using Lazarus+FPC right now.

[cpicanco]

Unfortunatelly, none of the proposed solutions worked for resolving the error. Right now I am explicitly handling empty strings this way:

Code: Pascal  [Select][+][-]

1. procedure TSimpleMsgPack.setAsString(pvValue: string);
2. begin
3.   FDataType := mptString;
4.   if SizeOf(Char) = 2 then
5.   begin
6.     SetLength(FValue, length(pvValue) shl 1);
7.     Move(PChar(pvValue)^, FValue[0], Length(FValue));
8.   end else
9.   begin
10.     if length(pvValue) > 0 then
11.     begin
12.       SetLength(FValue, Length(pvValue));
13.       Move(PChar(pvValue)^, FValue[0], Length(FValue));
14.     end else
15.     begin
16.       SetLength(FValue, 0);
17.     end;
18.   end;
19. end;

Or simply:

Code: Pascal  [Select][+][-]

1. procedure TSimpleMsgPack.setAsString(pvValue: string);
2. begin
3.   FDataType := mptString;
4.   if SizeOf(Char) = 2 then
5.   begin
6.     SetLength(FValue, length(pvValue) shl 1);
7.     Move(PChar(pvValue)^, FValue[0], Length(FValue));
8.   end else
9.   begin
10.       SetLength(FValue, Length(pvValue));
11.       if length(pvValue) > 0 then
12.           Move(PChar(pvValue)^, FValue[0], Length(FValue));
13.   end;
14. end;

[cpicanco]

PS.:  Will be testing using plain FPC by 11-09-2016.

[Fungus]

Why not do it like this?

Code: Pascal  [Select][+][-]

1. procedure TSimpleMsgPack.setAsString(pvValue: string);
2. var Len: Cardinal;
3. begin
4.   FDataType := mptString;
5.   Len:= Length(pvValue) * SizeOf(Char);
6.   SetLength(FValue, Len);
7.   if Len > 0 then Move(pvValue[1], FValue, Len);
8. end;

It is much sleeker and should not cause any problems :-)