logo
  • Buy
  • Download
  • Documentation
  • Blog
  • Contact

Blog

30 Mar 2015

FixInsight vs RTL

by Roman Yankovsky 3 Comments

In this post I will continue to analyze Delphi source code. Last episode was about Delphi VCL, in this post we will look into Delphi RTL. I will ignore minor issues and will try to find out the most interesting pieces of code. All code examples in this post are related to Delphi XE7 (version 21.0.17017.3725).

W503 Assignment right hand side is equal to its left hand side

  constructor TNativeDownloader.Create(PublicKey: string; Salt: array of Byte; MainFile, PatchFile: TApkFileInfo);
  begin
    FMainFile := MainFile;
    FPatchFile := FPatchFile;
    FDownloader := TJNativeDownloaderLauncher.JavaClass.init(StringToJString(PublicKey), ByteArrayToJArray(Salt),
      CreateApkList(MainFile, PatchFile));
    Create;
  end;

I think, it should be “FPatchFile := PatchFile”.

W501 Empty EXCEPT block

  try
    Assert(FConnections.Count = 0); // All connections should have been removed
  except
  end;

That’s not an issue, just a WTF moment :)

W517 Variable hides a class field, method or property

procedure TTask.ExecuteReplicates(Root: TTask);
var
                                     
//  ReplicasQuitting: Boolean;
  ReplicaProc: TProc;
begin
//  ReplicasQuitting := False;
  ReplicaProc := procedure
    var
      CurrentTask, ChildTask: ITask;
    begin
      CurrentTask := CurrentTask;
      if not Root.ShouldCreateReplica {or ReplicasQuitting} then
        Exit;
      ChildTask := Root.CreateReplicaTask(ReplicaProc, Self, [TCreateFlag.Replicating, TCreateFlag.Replica]);
      ChildTask.Start;
      try
        Root.CallUserCode;
      except
        Root.HandleException(TTask(ChildTask), TObject(AcquireExceptionObject));
      end;
    end;
  ReplicaProc;
end;

W517 is a very popular warning for Delphi VCL and RTL. But I find this one to be the most interesting. TTask class has a class method called CurrentTask and CurrentTask variable hides that method. By the way there is one more warning (line 1865): “W503 Assignment right hand side is equal to its left hand side”.

So should that assignment look like “CurrentTask := Self.CurrentTask” or is it just a useless variable? In any case that code looks weird.

I cannot quote all W517 warnings, there are too many of them. But let me quote one more example from VCL.

procedure TCustomTaskDialog.ShowHelpException(E: Exception);
var
  Msg: string;
  Flags: Integer;
  SubE: Exception;
begin
  Flags := MB_OK or MB_ICONSTOP;
  if Application.UseRightToLeftReading then
    Flags := Flags or MB_RTLREADING;
  // ... [skipped]

That’s not a bug. But it could cause bugs in the future if you change this code. For instance, this code will compile even if you remove Flags variable, because TCustomTaskDialog has a property with the same name – Flags. Also it’s too easy to mistype and use the variable in this method instead of the propety and vice versa.

W504 Missing INHERITED call in destructor

destructor TJavaImport.Destroy;
begin
  TJNIResolver.DeleteGlobalRef(GetObjectID);
end;

There are a number of missed “inherited” calls again, I quote only one though.

W508 Variable is assigned twice successively

      // Another thread could start traversing the list between when we set the
      // head to P and when we assign to P.Next.  Initializing P.Next to point
      // to itself will make others spin until we assign the tail to P.Next.
      P.Next := P;
      P.Next := PThreadInfo(AtomicExchange(Pointer(FHashTable[H]), Pointer(P)));

Not an issue, as far as I understood. But I think it worth quoting. I never thought that it is possible to do multithreading like this.

That’s all I have to bring today. If you like this post, please let me know what library or framework you wish to run FixInsight against at the next time. FMX? JEDI? Anything else?


  • Markus says:
    March 30, 2015 at 9:23 pm

    Of course checking FMX with that would be interesting!

    Reply
  • Stéphane WIERZBICKI says:
    March 31, 2015 at 11:25 pm

    FMX :D

    Reply
  • hinduism says:
    November 20, 2021 at 3:51 am

    Annette 7 With The Afterbeats Train Of Love https://wordpress.chiq.org.uk/94.html Manolo Escobar Y Sus Guitarras Madrecita Maria del Carmen

    Reply

Leave a Reply Cancel reply

Your email address will not be published. Required fields are marked *

  • Announcements
  • DelphiAST
  • DelphiSpec
  • FixInsight
  • FMX
  • Other
  • VCL

Recent Posts

  • Find leaks in Delphi with Deleaker
  • FixInsight and the inline directive
  • FixInsight 2017.04 support Delphi 10.2 Tokyo
  • FixInsight 2016.04 support Delphi 10.1 Berlin
  • FixInsight vs FMX in Delphi 10.1 Berlin

Archives

  • January 2020
  • April 2017
  • April 2016
  • March 2016
  • December 2015
  • November 2015
  • October 2015
  • September 2015
  • August 2015
  • April 2015
  • March 2015
  • February 2015
  • September 2014
  • August 2014
  • January 2014
  • December 2013
  • October 2013

Recent Comments

  • hinduism on FixInsight vs RTL
  • Christopher Monette on Using TCanvas in Delphi for Android
  • William Meyer on How to get a list of used units with DelphiAST
  • Edwardo on DelphiAST Announce
  • Https://Studybayhelp.Co.uk/ on Implementing LISP-like language in Delphi
  • Home
  • Buy
  • Download
  • Documentation
  • Blog
  • Contact
  • © 2014-2015 SourceOddity|
  • Terms and Conditions|
  • Privacy Policy