add a performance checker for const assignment

[WKPlus]

add a performance checker for situation like "const A a = getA()" and getA() returns const A & or A &

[danmar]

you also forgot to use astyle.

[PKEuS]

Can you please use Tokenizer::getFunctionTokenByName() instead of your own SymbolDatabase function?

In case that he needs a Function instance instead of a token, the new function is useful. However, some kind of funcId system similar to varIds might be a better solution (could handle overloads, namespaces, ...). The problem is just that we don't have it so far.

[danmar]

PKEuS: yes true

[kimmov]

And please squash those commits to one commit. You don't need to commit..revert..fix.. Just overwrite/squash the commit with new commit and force push to remote.

[WKPlus]

Moved checker/test cases into checkother/testother, reverted modifications of cppcheck.sln and testrunner.vcproj.
Run astyle and squash commits onto one.
Thanks so much for suggestions.
Git squash took up some time to understand and use, but when it works, I find it well-worth the time and trouble.

[WKPlus]

I found out why I run astyle failed the first time, I just run runastyle.bat in the root folder and did not check out the result before commitment. It's my fault:(
But there does not exist an 'astyle.exe' in the some folder which is called in runastyle.bat.
So, should we commit a astyle.exe in the root folder?

[kimmov]

No, external executables don't belong to Cppcheck repository. But the batch file should check if the astyle executable can be found from the path. I'm now on linux so can't do myself add the check in next days. But Stackoverflow has hints/suggestions how to do it:
http://stackoverflow.com/questions/4781772/how-to-test-if-executable-exists-in-path-inside-windows-batch-files

[PKEuS]

\n is missing here

[danmar]

The error message "Prefer reference than assignment for const variable" is a bit weird. To me it would sound better if "than" is replaced with "in". But it sounds like the assignment should be changed somehow and not the variable, doesn't it?

Perhaps: "variable 'a' can be reference to avoid data copy in assignment" or "changing 'a' to a reference avoids data copy". These are just ideas.

[WKPlus]

Actually I am not so clear whether assignment or variable be changed to reference?
If assignment, will "assignment can be reference for const variable 'a' to avoid data copying" be proper?
If variable, I prefer "variable 'a' can be reference to avoid data copy in assignment"

[kimmov]

Actually, back to basics.. Do you have any reference for your claim that your suggested code is more efficient? Is it just in some cases, or always etc? Is the performance difference measurable? In real-world code, not just in some micro-benchmarks?

[PKEuS]

As long as the message is only shown on user defined types, there is obviously a performance gain. It avoids copying data. (Just the same as parameters that are passed by value and not by reference). You usually don't see this message very often, so its nearly impossible to find a real world example.

[PKEuS]

Suggestions for a message:
"Type of 'a' should be changed to a const reference to avoid copying data."
"Ineffective/Useless/Redundant copying of data. Consider to change the type of 'a' to a const reference."

[WKPlus]

@kimmov
Actually, my first fixup in a performance tuning job is changing "A a = getA()" to "const A & a = getA()" where getA() returns const A & and variable 'a' will not be modified. And the fixup reduced the server response time from 22ms to 19ms and improved the cpu idle from 45.4 to 52.8.
Unfortunately the "A a = getA()" is not just the pattern "const A a = getA()" that we check, but I think it will still help us .
However, if we can detect such a pattern like "A a =" where 'a' will not be modified, it will help much more.

[WKPlus]

@PKEuS
According to your suggestion for error message, maybe you thought it's the variable not assignment should be change to reference.
So I prefer messages:
"Const variable 'a' can be reference to avoid data copying in assignment"
"Maybe redundant copying of data. Consider changing const variable 'a' to const reference."

If any suggestions, pls feel free to let me know.

[PKEuS]

Both ideas sound much better than the current message. But the word "Maybe" in the second one is unnecessary. It is redundant.

[WKPlus]

Changed the error message and the name of functions added.

[danmar]

That looks acceptable now as far as I see. If nobody complains I'll merge it tomorrow.

[danmar]

I took the liberty and edited your commit and pushed it.

I pushed it with:
188d2e1

Thank you for your work! I hope you'll provide more checks.