FixInsight vs FMX

In this post I will continue to analyze Delphi source code. Previous episodes was about Delphi VCL and RTL. As usual 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 XE8 (version 22.0.19027.8951).

W517 Variable hides a class field, method or property

procedure TLight.ReadDiffuse(Reader: TReader); var Color: Integer; begin IdentToAlphaColor(Reader.ReadIdent, Color); {$R-} Color := TAlphaColor(Color); {$R+} end;

The same issue in other unit.

procedure TStrokeCube.ReadDiffuse(Reader: TReader); var Color: Integer; begin IdentToAlphaColor(Reader.ReadIdent, Color); {$R-} Color := TAlphaColor(Color); {$R+} end;

That’s one of those “variable hides a class field” warnings I mentioned in previous post. This case is definetely a bug. TLight class, as well as TStrokeCube class, has Color property (it’s TAlphaColor, of course). So I guess it should be “Self.Color := TAlphaColor(Color)”, but better rename the variable. It’s a very bad style to use the same names for properties and variables. Usually it doesn’t harm, but sometimes leads to hard to find bugs. Just look at this code and don’t do that, ever.

W511 Object created in TRY block

begin if ([csDesigning, csDestroying, csLoading, csUpdating] * ComponentState <> []) or (FUpdating > 0) then Exit; { Update objects in form } try Comparer := TComparerTFmxObject.Create; ClientList := TTObjInfoList.Create(Comparer); Bucket := TDictionary<TObject, TObject>.Create; for InitiatedCount := 0 to 7 do begin if CollectActionClients(ClientList) = 0 then Break; ClientList.Sort; for I := 0 to ClientList.Count - 1 do ClientList[I].ActionClient.InitiateAction; ClientList.Clear; end; finally FreeAndNil(ClientList); FreeAndNil(Bucket); end; end;

Application may raise an exception before an object instance is actually assigned to ClientList and/or Bucket variables. That means the finally block below will try to free the memory which is unassigned.

W523 Interface declared without a GUID

IModelImporter = interface function GetDescription: string; function GetExt: string; function LoadFromFile(const AFileName: string; out AMesh: TMeshDynArray; const AOwner: TComponent): Boolean; end;

IFMXUISwitch = interface(UISwitch) { Touches } procedure touchesBegan(touches: NSSet; withEvent: UIEvent); cdecl; procedure touchesCancelled(touches: NSSet; withEvent: UIEvent); cdecl; procedure touchesEnded(touches: NSSet; withEvent: UIEvent); cdecl; procedure touchesMoved(touches: NSSet; withEvent: UIEvent); cdecl; procedure ValueChanged; cdecl; end;

Not a bug, actually. But since the interface does not have a GUID, it cannot be used with Supports function or with As operator. Maybe it worth adding a GUID, why not?

W510 Values on both sides of the operator are equal

function SamePosition(const APosition1, APosition2: TPosition): Boolean; overload; begin Result := (APosition1.X = APosition2.X) and (APosition1.Y = APosition1.Y); end;

I guess, it should be “APosition1.Y = APosition2.Y”.

W508 Variable is assigned twice successively

FPixelShader := TShaderManager.RegisterShaderFromData('gouraud.fps', TContextShaderKind.PixelShader, '', [ TContextShaderSource.Create(TContextShaderArch.DX9, [ $00, $02, $FF, $FF, $FE, $FF, $32, $00, $43, $54, $41, $42, $1C, $00, $00, $00, $9F, $00, $00, $00, $00, $02, $FF, $FF, $03, $00, $00, $00, $1C, $00, $00, $00, $00, $01, $00, $20, $98, $00, $00, $00, // skipped

FPixelShader := TShaderManager.RegisterShaderFromData('gouraud.fps', TContextShaderKind.PixelShader, '', [ TContextShaderSource.Create(TContextShaderArch.DX9, [ $00, $02, $FF, $FF, $FE, $FF, $32, $00, $43, $54, $41, $42, $1C, $00, $00, $00, $9F, $00, $00, $00, $00, $02, $FF, $FF, $03, $00, $00, $00, $1C, $00, $00, $00, $00, $01, $00, $20, $98, $00, $00, $00, // skipped

Two assignments in a row, probably not an issue, just a sloppy copy-paste.

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

if X1 > X2 then X1 := X1; if Y1 > Y2 then Y1 := Y2;

I guess, it should be “X1 := X2”.

if (Self.Form <> nil) and (Self.Form.Handle <> nil) then Self.Form := Self.Form;

Not sure what was meant to be there (Self.Form is a record field).

And one more.

if FNew.FFrequency <> 0 then FNew.FValue := Round(FNew.FValue / FNew.FFrequency) * FNew.FFrequency else FNew.FValue := FNew.FValue;

W510 Values on both sides of the operator are equal

if RegionSize = RegionSize then begin SetLength(UpdateRects, RegionData.rdh.nCount); for i := 0 to RegionData.rdh.nCount - 1 do begin R := PRgnRects(@RegionData.buffer[0])[i]; UpdateRects[i] := RectF(R.Left, R.Top, R.Right, R.Bottom); end; end;

Not sure what was meant to be there.

W513 Format parameter count mismatch

function TCustomValueRange.GetNamePath: string; begin Result := Format( 'Value: %0:*.*f (%1:*.*f..%2:*.*f)', [Value, Min, Max]); end;

The format string is incorrect. This code will raise an exception.

W505 Empty THEN block

if FoundValue.Count > 1 then else if FoundValue.Count > 0 then PropValues[Name] := FoundValue[0];

This looks strange, perhaps it could be replaces with a simple “If FoundValue.Count = 1 then PropValues[Name] := FoundValue[0]”.

Well, that’s all I have to bring today. Use FixInsight to find bugs in your code before your customers do :)