#1 2015-12-07 11:36:57

esmondb
Member
From: London
Registered: 2010-07-20
Posts: 299

FixInsight

I don't normally use IDE plug-ins but has anyone found FixInsight useful?

Trying it out on mormot.pas doesn't seem to bring up any serious issues.

There are a few empty else blocks and a couple of missing 'inherited;' statements in overriden constructors which I assume are harmless.

And these warnings which I also assume are harmless:

[FixInsight Warning] mORMot.pas(34128): W509 Unreachable code
[FixInsight Warning] mORMot.pas(48157): W509 Unreachable code
[FixInsight Warning] mORMot.pas(49273): W515 Suspect FREE call

Offline

#2 2015-12-07 12:50:50

ab
Administrator
From: France
Registered: 2010-06-21
Posts: 14,666
Website

Re: FixInsight

We get rid of some unneeded inherited calls, e.g. when they indeed call the void TObject.Create constructor.
They compile as a plain call + ret assembly. That is, breaking the CPU execution flow for nothing.
I assume FixInsight should better safely ignore the fact that TObject.Create constructor is not called.
A virtual constructor should always be called via inherited, but for a static void method, as part of the RTL as TObject.Create, it is a bit too paranoid.

And yes, all those are harmless, or even expected to be as such.
I've fixed one or two by http://synopse.info/fossil/info/2c7c383760

We fixed today http://synopse.info/fossil/info/aa468640c592a which was, on the other hand, a more serious error.
In practice, it did no harm (just executing some code twice), but the code flow was clearly wrong.

Feel free to report more detailed doubts here!
Using static analysis as proposed by FixInsight is a good idea.
Did you test also SynCommons, or other framework units?

Offline

#3 2015-12-07 14:11:54

esmondb
Member
From: London
Registered: 2010-07-20
Posts: 299

Re: FixInsight

Thanks for the extra insight! I haven't had time to test any other units.

Offline

Board footer

Powered by FluxBB