Jump to content
Sign in to follow this  
Czar

TryStrToInt bug

Recommended Posts

I have found a most strange bug.

 

If I use TryStrToInt then the integer variable is not accessible anywhere else in the program except for in the current local procedure.

 

Here is the code. You can see the difficulty is a global. As long as the line

TryStrToInt(d, difficulty);

remains in the program as valid code the difficulty will not available in other procedures. I put the d='20' to show you that the code doesn't even need to run for it to screw up the results.

 

If you run the code you will see in the console that difficulty is 101 in the procedure but when the writeln in GetQuestion is run the console shows [object object].

 

If you comment out TryStrToInt (and preceding line) then everything works as expected.

 

unit Form1;

interface

uses 
  SmartCL.System, SmartCL.Graphics, SmartCL.Components, SmartCL.Forms, system.types,
  SmartCL.Fonts, SmartCL.Borders, SmartCL.Application;

type
  TForm1 = class(TW3Form)
  private
    {$I 'Form1:intf'}
    procedure GetQuestion;
  protected
    procedure InitializeForm; override;
    procedure InitializeObject; override;
    procedure Resize; override;
  end;

var difficulty : integer;

implementation

{ TForm1 }

procedure TForm1.InitializeForm;
begin
  inherited;
  // this is a good place to initialize components

  var d : string;
  d := '10';

  difficulty := 101;

  if d='20' then  // only here to prove a point
  TryStrToInt(d, difficulty);

  writeln('Difficulty exists here >'+inttostr(difficulty));

  GetQuestion;
end;

procedure TForm1.GetQuestion;
begin
  writeln('But difficulty no longer existss here > '+inttostr(difficulty));  // result = [object object]
end;

procedure TForm1.InitializeObject;
begin
  inherited;
  {$I 'Form1:impl'}
end;
 
procedure TForm1.Resize;
begin
  inherited;
end;
 
initialization
  Forms.RegisterForm({$I %FILE%}, TForm1);
end.

Share this post


Link to post
Share on other sites

And, it is not optimized away becuase if you put another writeln after the GetQuestion call same result.

 

 

writeln('Difficulty exists here >'+inttostr(difficulty));

 

GetQuestion;

 

writeln('Difficulty exists here >'+inttostr(difficulty));

Share this post


Link to post
Share on other sites

Look at the signature of the function TryStrToInt(Data: string; var Value: integer): boolean;.

Hum, there's a variable to be passed by reference with the var prefix.

Pass by reference means that the subroutine actually refers to the passed variable rather than its value. This is OK.

 

in SmartMS, when we declare a global variable to pass by reference

var difficulty : integer = 0;
underneath the cloths, SMS compiler will emit an object with the hidden special field v

  var difficulty = {};  
  difficulty.v = 0;
Let's do an experiment:

procedure DoIt(var A : Integer);    
begin  
  A := A * 2;      
  console.log(Format('difficulty in the procedure = %d',[A]));
end;

procedure TForm1.InitializeForm;
begin  inherited;    
  difficulty := 22;    
  console.log(Format('difficulty in program before call = %d',[difficulty]));   
  // Call the procedure DoIt
  DoIt(difficulty);     
  console.log(Format('difficulty in program now = %d',[difficulty]));
end;
The result is:

difficulty in program before call = 22

difficulty in the procedure = 44

difficulty in program now = 44

 

Look, the caller "difficulty" variable is updated by the procedure DoIt. This is expected.

This is a very useful way of returning data from a procedure.

 

...when we try to retrieve the variable difficulty directly in SmartMS,

humm, the "secret variable" was stored in a weird field called v in an object called 'difficulty'.

We can access this secret value in the JS object directly, an workarount would be:

 

procedure TForm1.GetQuestion;
begin    
  console.log( variant(difficulty).v );  
end;
/* or declare a global external variable */

function dificuldate : Variant; external 'difficulty.v' property;

 

procedure TForm1.GetQuestion;
begin    
  console.log(dificuldate ); 
end;

Share this post


Link to post
Share on other sites

Thanks for the response, I think I follow it.

 

However, it is still a significant bug. Any normal programmer used to Delphi would expect TryStrToInt to return a 0 if it failed.

 

But in this case the weird effect s even worse because the TryStrToInt screws up the integer variable even if it is not executed!

 

----------

 

For example I only want to run TryStrToInt if I have an incoming parameter

 

e.g., if pos('?', BrowserAPI.Window.location.href)>0 then

begin

var query : variant; Query :=parseUrlQuery(BrowserAPI.Window.location.href);

TryStrToInt(query.difficulty, d);

end;

 

If the pos results in zero - i.e., don't run the TryStrToInt - it still screws up the variable even though it was never run.

 

I would expect TryStrToInt to work exactly like the Delphi version, i.e., come back with a zero if the Str To Int failed.

 

-------------

 

I have now come across a number of weird behaviours where SMS does not function as expected. I do not think it is suitable for these issues to be swept under the carpet as it means it is not possible to trust SMS to do what is required. :( These unexpected behaviour issues certainly make debugging more difficult as you cannot assume the underlying code is doing what you are expecting.

Share this post


Link to post
Share on other sites

Apparently, this piece of code is working for me;

 

http://localhost/projMICRO/www/index.html?abc=123

 

  var
    vTexto : String;
    vNumero : Integer;
  begin

    if (document.location.search.indexOf('abc=') >= 0) then
      vTexto:= document.location.search.split("abc=")[1] else exit;

    if TryStrToInt(vTexto, vNumero) then
      console.log('Success conversion! Value: ' + IntToStr(VNumero))
    else
    begin
      console.log('No possible to convert ' + vTexto + ' to integer');
      console.log(vNumero);
    end;

Share this post


Link to post
Share on other sites

Why?

 

Because TryStrToInt would be my go to method in Delphi

 

Because I have never used StrToIntDef.

 

I am sure it is not intended but your answer Jarto comes across as if it obvious that I should be using this StrToIntDef. I don't believe it addresses the issue that TryStrToInt is broken. If the familiar parts of SMS diverge from Delphi too much it makes it very frustrating.

Share this post


Link to post
Share on other sites

This is not a bug, but rather a missunderstanding of how JSVM deals with inline values.

Variables that are created inline (ex: var a := 10) exist only in local scope (so in your case they die when you pass them as a var param). What happens here is that you are pushing a value out of scope, at which point the garbage collector eats it, and you get zero in return.

 

I know this is quite confusing, but that's JavaScript for you.

 

The best way to ensure "delphi like" results is to avoid using "JS like" features.

Inline variables etc. is great for fast calculations and temporary values that doesnt leave the scope - or where you directly assign the value to a class field. But using them with var-param based procedures is a throw of the dice.

 

Define the variables "the old way" and it should be ok :)

Share this post


Link to post
Share on other sites

 

@Cipher I think you are mistaken the variable is a global. Also the fact that it breaks the variable even when not executed is a worry.

 

 

I had a closer look at now I was able to reproduce the problem.

Yes, it seems that this combination was not implemented by the codegen. The way compilers work is that there has to be a unified way to deal with source/target symbols. In this case the combination of a global variable mixed with a class-context call to a unit procedure somehow returned the whole var object rather than just the value.

 

Not a huge bug but indeed enough to cause frustration.

I will pass this along to Eric and see what we can do.

 

The codegen has a test-pass rate of 95% which is actually very high (Delphi XE6 had a rate of 70+ before the updates), but like all living software things can break. And ofcourse, we are very sorry that we didnt pick up on these before. The Smart Mobile Studio codebase is a massive undertaking, and there are bound to be glitches and mistakes.

 

Smart has remained somewhat in the shadows for a couple of years, and we have finally started to get the wheels turning again. So while there will be bugs (especially now in the Alpha phase) - we are here to remove them.

 

So keep on reporting things like this and we will fix them one by one. In a perfect world we would have caught this sooner, but like I mentioned earlier - a lot of this tech doesnt exist in JavaScript. So we have had to work miles off the beaten track. No body has done anything like this before, so there will be odd workarounds here and there.

 

Best regards

 

Jon L. Aasenden

Share this post


Link to post
Share on other sites

I have tracked the bug down, and its actually not about TryStrToInt.

The bug happens even if you remove calls to that function.

 

It is the codegen that has made a mistake with regards to how values are stored inside SMS. The {}.v thing is the only way to emulate VAR params, and for some odd reason that is not dealt with properly.

 

But I have registered the bug with the guy doing the codegen. Hopefully that will be resolves quickly.

Share this post


Link to post
Share on other sites

@Cipher/Jon I am glad I persisted :) We are putting a heap of hours into coding projects that we are using in real-world situations which is a great way to test the alpha so we (RecursiveElk and I )are really pleased to be able to provide feedback when we find weird and odd behaviour. We certainly don't expect a smooth ride at the moment.

Share this post


Link to post
Share on other sites
On 6/14/2019 at 1:30 AM, Czar said:

Ran into this issue again - who is looking after the codegen?

Sorry for taking a long time to answer this. We're working on upgrading the compiler to use the newest DWS. While doing that, I made a quick test. Unfortunately this bug is still there. So we need to dig deeper.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
Sign in to follow this  

×