www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - FWIW: results of cppcheck on dmd sources

reply Trass3r <un known.com> writes:
http://trass3r.github.com/dmd-cppcheck/

Seems like the tool isn't dead after all so I tried it out with the dmd  
sources.
Some false positives but also some valid points.

E.g. http://trass3r.github.com/dmd-cppcheck/15.html#line-960 looks  
particularly interesting.
Jun 19 2012
next sibling parent "bearophile" <bearophileHUGS lycos.com> writes:
Trass3r:

 http://trass3r.github.com/dmd-cppcheck/


Beside the usual "Unused variable: xxxx", and the nice "Variable 
'xxx' is assigned a value that is never used", I see an uncommon 
class of suggestions, "The scope of the variable 'xxxxx' can be 
reduced." that is simple but useful.

Bye,
bearophile
Jun 19 2012
prev sibling next sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 6/19/2012 7:05 AM, Trass3r wrote:

 http://trass3r.github.com/dmd-cppcheck/

 Seems like the tool isn't dead after all so I tried it out with the dmd
sources.
 Some false positives but also some valid points.

 E.g. http://trass3r.github.com/dmd-cppcheck/15.html#line-960 looks particularly
 interesting.


Thanks for posting this. I think these are worth correcting, although in my 
quick scan I didn't see any actual bugs.
Jun 19 2012
parent reply "Brad Anderson" <eco gnuk.net> writes:
On Tuesday, 19 June 2012 at 17:10:03 UTC, Walter Bright wrote:

 On 6/19/2012 7:05 AM, Trass3r wrote:

 http://trass3r.github.com/dmd-cppcheck/

 Seems like the tool isn't dead after all so I tried it out 
 with the dmd sources.
 Some false positives but also some valid points.

 E.g. http://trass3r.github.com/dmd-cppcheck/15.html#line-960 
 looks particularly
 interesting.


 Thanks for posting this. I think these are worth correcting, 
 although in my quick scan I didn't see any actual bugs.


CppCheck is actually how I found the two bugs I fixed recently 
(CtfeStack::maxStackPointer and ComplexExp::toChars).  I spent a 
couple hours going through the results of CppCheck but only found 
those two actual problems (plus the third Globals thing that 
turned out to not be a problem).  That's not to say I thoroughly 
investigated every item.  There sure are a lot of unused 
variables in DMD :P.  I didn't feel qualified to submit a pull 
request removing them as I don't have a very strong understanding 
of DMD's source code yet.

One non-bug I thought about addressing but didn't end up doing 
was this:

http://trass3r.github.com/dmd-cppcheck/18.html#line-4259

I looked up the file history and it seems like that's just a 
result of Don doing some refactoring.

There are surprisingly few notices of memory leaks considering 
DMD uses a GC.

Regards,
Brad Anderson
Jun 19 2012
next sibling parent reply Trass3r <un known.com> writes:
 There are surprisingly few notices of memory leaks considering DMD uses  
 a GC.


This is news to me. Has the GC been re-enabled?
Jun 19 2012
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 6/19/2012 10:54 AM, Trass3r wrote:

 There are surprisingly few notices of memory leaks considering DMD uses a GC.


 This is news to me. Has the GC been re-enabled?


No, it's just that cppcheck doesn't (and cannot) detect global memory leaks - 
only very localized ones.
Jun 19 2012
parent reply Trass3r <un known.com> writes:
 This is news to me. Has the GC been re-enabled?


 No, it's just that cppcheck doesn't (and cannot) detect global memory  
 leaks - only very localized ones.


btw, what is the stance on adding some manual memory freeing, at least for  
obvious local leaks?
Jun 19 2012
parent Walter Bright <newshound2 digitalmars.com> writes:
On 6/19/2012 1:41 PM, Trass3r wrote:

 This is news to me. Has the GC been re-enabled?


 No, it's just that cppcheck doesn't (and cannot) detect global memory leaks -
 only very localized ones.


 btw, what is the stance on adding some manual memory freeing, at least for
 obvious local leaks?


I think it's fine, but it has to be done carefully to avoid bugs.
Jun 19 2012
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 6/19/2012 10:29 AM, Brad Anderson wrote:

 CppCheck is actually how I found the two bugs I fixed recently
 (CtfeStack::maxStackPointer and ComplexExp::toChars). I spent a couple hours
 going through the results of CppCheck but only found those two actual problems
 (plus the third Globals thing that turned out to not be a problem). That's not
 to say I thoroughly investigated every item. There sure are a lot of unused
 variables in DMD :P. I didn't feel qualified to submit a pull request removing
 them as I don't have a very strong understanding of DMD's source code yet.


I think in general fixing cppcheck's complaints makes the code more 
understandable and maintainable, even if it doesn't actually fix anything.

A lot of the cruft in the code comes from the endless turmoil it undergoes :-)
Jun 19 2012
parent Trass3r <un known.com> writes:
 I think in general fixing cppcheck's complaints makes the code more  
 understandable and maintainable, even if it doesn't actually fix  
 anything.


+1
Jun 19 2012
prev sibling parent Trass3r <un known.com> writes:
http://trass3r.github.com/dmd-clang/
Jun 19 2012