FixInsight 2017.04 introduces rule O805 “Inline marked routine comes after its call in the same unit”. Let’s check FMX and VCL code to see if Embarcadero follows their own rules.
FixInsight 2017.04 introduces rule O805 “Inline marked routine comes after its call in the same unit”. Let’s check FMX and VCL code to see if Embarcadero follows their own rules.
http://sourceoddity.com/blog/2017/04/fixinsight-and-the-inline-directive/
http://sourceoddity.com/blog/2017/04/fixinsight-and-the-inline-directive/
Did not know this... Interesting to see when we get our FixInsight updated ;)
ReplyDeleteThanks man!
Roman Yankovsky - Is there a "code after raise" warning? I stumbled upon two code segments the other day, which did a object.free in code just after a raise - i.e. code that would never run due to the raise.
ReplyDeleteLars Fosdal Yes, there is "W509 Unreachable code."
ReplyDeleteLars Fosdal The call to Free should have been in a finally block. That was the real problem I guess. Then the raise presents no trouble.
ReplyDeleteDavid Heffernan There were conditions involved. Isn't it fun to speculate about you have never seen?
ReplyDeleteThere's a simple pattern to follow. Once you follow it, with Try/Finally, you can't go wrong. If your object isn't protected by a finally block, as must be the case, surely that is a mistake.
ReplyDeleteDavid Heffernan Yeah, surely any factory needs to destroy what it creates.
ReplyDeleteThe pattern is simple. Try/Finally we know. The alternative is in a destructor, and they are contracted not to raise. Very few exceptions. The call in interface Release springs to mind.
ReplyDeleteWell, I ain't about to rewrite the code. Way too complex. Moving the .Free before the raise, solved the problem. Having a tool that can spot similar issues is nice.
ReplyDeleteStill have issues with ignoring unused constants. Any news on a fix - Roman Yankovsky ??
ReplyDeleteTommi Prami Sorry for delaying the response. I'm going to release an update during the next week. By the end of a week, perhaps.
ReplyDeleteRoman Yankovsky Thanks for the info, hope we have new release in about week...
ReplyDeleteTommi Prami Please give it a try sourceoddity.com - sourceoddity.com/download/FixInsight_2017.04upd1_setup.exe
ReplyDeleteRoman Yankovsky At least my test show that I can ignore unused constant. But weird thing was that in Config we had globally removed the O803, but it still kept complaining after installation. Could thee be bug. How to ensure that the config file is actually used. (Log??)
ReplyDeleteRoman Yankovsky
ReplyDeletecouple (pseudo code) suggestions, some lead to way too long lines and so
1. Parameter ignores are way too picky of their position and also makes it syntactically slightly difficult to maintain
function Foo (Foo1, Foo2, Foo3, Foo4, Foo5): Boolean;
//FI:ToomenayParamsCode
//FI:ParamDefinedNotUSedCode
{ later
//FI:ParamDefinedNotUSedCode(Foo4) }
var
I: Integer
begin
..
2. try except ignore little too picky of position (IMHO)
try
..
except
//FI:IgnoreEatingException
end;
3. Add warnign for unneeded FixInsigh ignore.
Maybe something was ignored in past but not needed anymore.
due refactoring etc. Like there was too many variables,
but after refactoring there is not too many, but no one has removed
the ignore.
4. Maybe more verbose ignoring would be better, Current //FI:O666 -> //FI:DevilIshErrorIgnored Similarly as there are compiler directives, shot and verbose versions.
Just my 0.02€
Tommi Prami In Delphi go to Help -> About FixInsight and check 'Debug mode' checkbox. Then run FixInsight. It will create .filog file in your project directory. Please send it to roman@tmssoftware.com
ReplyDeleteI don't have local installation...
ReplyDelete