Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False positives in VariableInitialization around SetLength intrinsic #250

Closed
zedxxx opened this issue Jun 6, 2024 · 6 comments
Closed
Assignees
Labels
invalid This doesn't seem right

Comments

@zedxxx
Copy link

zedxxx commented Jun 6, 2024

Delphi IDE version: Delphi 12.1

DelphiLint version: 1.1.0

SonarDelphi version: 1.6.0

1

unit Unit1;

interface

uses
  GR32,
  GR32_Polygons,
  GR32_VectorUtils;

procedure DrawThickLine(ABitmap: TBitmap32; const AStart, AEnd: TFloatPoint; const AColor: TColor32;
  const AWidth: Integer); inline;

procedure DrawThickPolyLine(ABitmap: TBitmap32; const APoints: TArrayOfFloatPoint; const AColor: TColor32;
  const AWidth: Integer);

implementation

procedure DrawThickLine(ABitmap: TBitmap32; const AStart, AEnd: TFloatPoint; const AColor: TColor32;
  const AWidth: Integer);
var
  VLine: TArrayOfFloatPoint;
begin
  SetLength(VLine, 2);

  VLine[0] := AStart;
  VLine[1] := AEnd;

  DrawThickPolyLine(ABitmap, VLine, AColor, AWidth);
end;

procedure DrawThickPolyLine(ABitmap: TBitmap32; const APoints: TArrayOfFloatPoint;
  const AColor: TColor32; const AWidth: Integer);
var
  VPoly: TArrayOfFloatPoint;
begin
  VPoly := BuildPolyLine(APoints, AWidth);

  if not ABitmap.MeasuringMode then begin
    PolygonFS(ABitmap, VPoly, AColor);
  end;

  ABitmap.Changed(MakeRect(PolygonBounds(VPoly), rrOutside));
end;

end.
@Cirras
Copy link
Collaborator

Cirras commented Jun 6, 2024

Hi @zedxxx, thanks for reporting these!

The first issue (uninitialized VLine) might be a mistake in the way we're modeling the SetLength intrinsic.
Definitely a problem - I'll have a look at fixing it.

The second issue (unused VPoly) looks like a name resolution issue on the PolygonFS invocation - I'm interested in chasing it, but can you make a new GitHub issue to track that distinct bug separately?

@Cirras Cirras changed the title False positives about unused variable and variable initialization False positives in VariableInitialization around SetLength intrinsic Jun 6, 2024
@Cirras Cirras self-assigned this Jun 6, 2024
@zedxxx
Copy link
Author

zedxxx commented Jun 6, 2024

The second issue (unused VPoly) looks like a name resolution issue on the PolygonFS invocation - I'm interested in chasing it, but can you make a new GitHub issue to track that distinct bug separately ?

Sure: #251

@Cirras
Copy link
Collaborator

Cirras commented Jun 6, 2024

Cheers, I'll chase up these issues.
Thanks for reporting them. 👍

@Cirras
Copy link
Collaborator

Cirras commented Jun 11, 2024

Hi @zedxxx,

Looks like SetLength is modeled correctly - and I can't seem to reproduce this VariableInitialization FP.
(See image below)

Here's what I did:

  • created a new VCL project
  • cloned the Graphics32 library and added the Source folder to the search path in project options
  • added your Unit1 file to the project
  • built the project successfully (just to make sure it could compile)
  • scanned with DelphiLint (FP issue not reproduced)
    image

The analyzer requires source code to perform type & invocation resolution, and things can go quite haywire if dependency source code isn't in the search path (or debugger source path).

One potential thing - do you only have the Graphics32 DCUs on your search path, and not the source files?

@zedxxx
Copy link
Author

zedxxx commented Jun 11, 2024

In my case, the path to the sources was added through the Delphi settings, not the project settings. Also, the IDE environment variable is used to specify the path (if it matters).

1

@Cirras
Copy link
Collaborator

Cirras commented Jun 11, 2024

Ah, there's the problem.

The analyzer doesn't currently read the library path. Adding support hasn't been seriously looked at, but it would certainly improve the accuracy of our import resolution (as evidenced by your case).

In your case, for an accurate analysis you'll have to add your dependencies to the search path or debug source path.

We could look into adding support for the library path, and you're welcome to open a feature request.

@Cirras Cirras added invalid This doesn't seem right and removed bug Something isn't working labels Jun 11, 2024
@Cirras Cirras closed this as not planned Won't fix, can't repro, duplicate, stale Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants