LCA: Static analysis with GCC plugins

Posted Jan 23, 2010 0:22 UTC (Sat) by tialaramex (subscriber, #21167)
In reply to: LCA: Static analysis with GCC plugins by jreiser
Parent article: LCA: Static analysis with GCC plugins

A great many things that are a bad idea in general are, sometimes, useful or even necessary. All the "low hanging fruit" has been picked - stuff like "printf("%d and %d", number, string)" is now captured without any special tools. So you're left with things that might be correct, but probably aren't.

Even some of that gets warned about, some classic beginners mistakes with precedence in C will get reported as warnings by compilers these days - if you really wanted that precedence you need merely add parentheses making it clear (to the compiler AND the poor maintenance programmer coming along after you) to shut up the warning.

LCA: Static analysis with GCC plugins

Posted Jan 23, 2010 23:04 UTC (Sat) by ikm (guest, #493) [Link] (2 responses)

> So you're left with things that might be correct, but probably aren't.

C++ is naturally quite context-dependent. And shadowing in it is quite useful -- you don't need to come up with some crazy variable names just because all the saner ones are already used. I shadow stuff all the time and it never really backfired. And I do enjoy nice, concise, context-dependent names. So I don't share the "probably aren't" part.

LCA: Static analysis with GCC plugins

Posted Jan 25, 2010 10:34 UTC (Mon) by epa (subscriber, #39769) [Link] (1 responses)

I guess the tool could give you a warning for the unintentional cases, and when you really do want to shadow a class member you can flag it with some new keyword or magic comment:

int x; /* shadow */

LCA: Static analysis with GCC plugins

Posted Feb 4, 2010 12:09 UTC (Thu) by adobriyan (subscriber, #30858) [Link]

> int x; /* shadow */

Don't overload comments.

#ifdef __CHECKER__
#define __shadow ...
#else
#define __shadow
#endif

int __shadow x;

LCA: Static analysis with GCC plugins

Posted Jan 25, 2010 17:42 UTC (Mon) by iabervon (subscriber, #722) [Link] (2 responses)

I'd actually say that the goal is finding things that are possibly correct but misleading. It is well-defined what happens if you shadow a variable in C++, but someone who encounters a variable reference two pages from the start of a method that matches one of the method arguments is likely to overlook the fact that there's a shadowing declaration on the middle page.

Similarly, the precedence of shifts is unintuitive when combined with addition and subtraction; a line that combines these without parentheses is likely to either get the wrong value, or to confuse people.

Static analysis tools can do things better than language definitions in this area, too. A static analysis tool can decide whether to warn about something based on the textual size of the scope in which a variable is shadowed, representing an approximation of the likelihood of people not seeing the shadowing declaration, which is not something the language definition should be trying to estimate.

Shadowing a class member: wrong?

Posted Jan 29, 2010 16:35 UTC (Fri) by giraffedata (guest, #1954) [Link] (1 responses)

I'd say an even more basic goal is to provide the coder with tools to assist in his personal coding style. If you like to use local variables that shadow class members, fine. Don't turn on this checker. But if you avoid that because you find you do it wrong too often, turn on the checker.

Personally, I wouldn't use this, but would really like to see a checker that catches every time I refer to a class member implicitly (i.e. without "this->". To prevent confusion about what is a class member and what is local, I observe a discipline of always referring to the former with this->, but get burned sometimes when I neglect to define a local variable and implicitly get a class member instead.

Shadowing a class member: wrong?

Posted Jan 29, 2010 17:55 UTC (Fri) by iabervon (subscriber, #722) [Link]

Obviously, it has to be on a project scale instead of an individual scale; it doesn't help if you never make a mistake based on a variable shadowing, if it's not common in a particular project and other people make mistakes because of that code.

In general, it's probably best to either always use "this->" or never use it (and never shadow members) within a given codebase. Either way works, but any mixture leads to patches which can't be reviewed easily (that is, you can't tell from the function header and 3 lines of context whether a patch is right).