[PATCH][StaticAnalyzer] new check comparing equal expression

[Jordan Rose]

On Sep 26, 2013, at 2:30 , Daniel Marjamäki <Daniel.Marjamaki at evidente.se> wrote:

> 
> Hello!
> 
> Thanks for your reviews.
> 
> I have seen a few comments about using "const". I am confused about this. To me it seems that const is avoided throughout llvm/clang.
> 
> Can somebody clarify for me when it's appropriate to use const?
> 
> Is "const" ok in the analyzer but not in other parts of llvm/clang?
> 
> Should const be used for function arguments also or is it just for auto variables?

I personally like to use const on all pointers where I don't intend to modify the thing, especially for AST nodes. A lot of times parts of Sema do end up modifying something in the AST, so the pointees can't be marked const. The analyzer, though, can never modify the AST, so we always prefer using const. (And since I'm a reviewer for the analyzer, I can actually enforce that in the analyzer. Not sure if there's a hard rule in Sema or CodeGen.)

There are also bits of AST (and other libraries) that are not quite const-correct, usually because no one bothered. Patches to fix those cases are usually welcome.


> Should a function be const if it has no side effects? For instance:
> class X {
> public:
>    int get123() const { return 123; }
> };

A member function should be const if it makes sense for someone with a const reference to call it. Checker callbacks in particular are const to remind you that the callbacks may occur out of order and so state should generally not be kept in the Checker itself.


> Should a non-pointer be const? For instance:
> void f() {
>    const unsigned NumArgs = Args.size();
>    .. do various stuff, the intention is that NumArgs should not be changed ..
> }

This one I think we generally don't bother with. If the object's not shared, there's no danger in accidentally messing it up. It does declare a bit of intent, though, so I wouldn't ask you to remove it.

Thanks for asking about this; the respect for our coding style and practices is much appreciated.

Jordan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130926/0488ed87/attachment.html>