Ticket #6143 (closed: fixed)
CPP check investigation
| Reported by: | Nick Draper | Owned by: | Michael Reuter |
|---|---|---|---|
| Priority: | critical | Milestone: | Release 2.4 |
| Component: | Mantid | Keywords: | |
| Cc: | Blocked By: | ||
| Blocking: | Tester: | Owen Arnold |
Description
From taking a look at the CPP check output and the code suggest a way forward for all of the development team to follow.
e.g. What files we should not interfere with (can these be excluded?) What level of warnings we should consider fixing, both in new code and historic.
With your recent work on these you probably have most of the info you need already
Change History
comment:2 Changed 8 years ago by Michael Reuter
My initial reaction is that we should be striving for zero errors of any kind from our static analysis tools. That being said, all items with severity error, information and performance should be dealt with immediately. We've picked up a bunch of performance errors dealing with what basically boils down to unused return values from functions. There is a way we could handle that which I'll speak to in another entry.
The style errors breakdown into two broad categories: C style casts and unread/unused variables. This is basically programmer laziness/apathy. The former is a violation of our coding standards spelled out here in point 2. The latter should be easily fixable. Now, the former (C style casts) appears a lot in the ingested qtiplot code. In principle, we could just let these go, but while going through my last cppcheck squashing round and some unrelated poking around, I noticed Mantid developers had placed old style casts in that code to avoid warnings. If we're going to touch code, then it should conform to our coding standards.
The warning errors are mostly unfinished/partial initialization lists in constructors. This is also a violation of the coding standard shown here in point 4. However, it looks like nearly all the errors come from qtiplot code, so we'll have to decide if we go the "it's our code" route.
comment:3 Changed 8 years ago by Michael Reuter
- Status changed from accepted to verify
- Resolution set to fixed
For handling the unused return values from function, one thought is to fix them according to the prescription laid out here. Russell has already voiced concern about possible performance impacts. I suppose we could actually use and check the return values like they are intended.
comment:4 Changed 8 years ago by Owen Arnold
- Status changed from verify to verifying
- Tester set to Owen Arnold
This looks to be a f
