llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.31k stars 12.11k forks source link

Add Clang-Tidy checks for localizing variables #22366

Open EugeneZelenko opened 9 years ago

EugeneZelenko commented 9 years ago
Bugzilla Link 21992
Version unspecified
OS All
CC @LegalizeAdulthood

Extended Description

Hi!

It'll be great to implement suggestion for localizing variables.

It's simpler in case of conditional statements, where variable is used in one of branches. It's harder to do in case of loops, when variable may be re-initialized on each iteration or accumulate value with each iteration.

Background of request: my project have quite a lot long functions (~ 100 kB of code) where place of usage and life cycle of variables are hard to track.

Eugene.

LegalizeAdulthood commented 9 years ago

This comes up all the time when working with legacy code and very large functions, particularly code written in a C style where all variables are declared at the top of the function. (Ironic, because even C lets you declare variables locally to any block scope.) It becomes nearly impossible to identify which variables are used only in a local block of code when it is written in this style.

In my experience in transforming such code, there are a number of very useful transformations that can be applied to localize variable dependencies among statements in order to make sense of the code before applying Compose Method[1] to the long method:

  1. Split all multiple declarations into single declarations: int i, j = 1, k, *p = NULL;

    =>

    int i; int j = 1; int k; int *p = NULL;

  2. Split reuse of temporaries into distinct variables: int i = 1; // use i // many lines of code not using i i = 2; // use i // many lines of code not using i

    =>

    int i = 1; // use i // many lines of code not using i int i2 = 2; // change uses of i to i2 // many lines of code not using i or i2

  3. Localize variables

This request covers the third transformation, but once you have this and you're facing such "monster functions" in legacy code, you really want the first two as well. Without the splitting of reused temporaries, Extract Method ends up including all kinds of variables in the argument list that are in fact only locally used. Presumably any tool that can localize a variable to its first use would also have to handle munging up a declaration of multiple variables in a single statement if the variable to be moved occurred in such a declaration. However, this is very useful in its own right in more situations than just dealing with the monster function/method.

[1] Compose Method: http://www.industriallogic.com/xp/refactoring/composeMethod.html

EugeneZelenko commented 9 years ago

Bug llvm/llvm-project#22149 has been marked as a duplicate of this bug.

EugeneZelenko commented 9 years ago

In other Parasoft C/C++test check for C++ (OPT-20-3) it try to push variable declaration to place of usage in same block to avoid unnecessary destructors calls.

EugeneZelenko commented 9 years ago

Parasoft C/C++test has this check (JSF-136_b-4).

EugeneZelenko commented 9 years ago

In C++ loop iterators variables could be localized too:

static void LoopIterator() { int i;

for (i = 0; i < 10; ++i) ;

std::vector Vector; std::vector::const_iterator j; std::vector::const_iterator End;

for (j = Vector.begin(), End = Vector.end(); j != End; ++j) ; }

static void LoopIteratorFixed() { for (int i = 0; i < 10; ++i) ;

std::vector Vector;

for (std::vector::const_iterator j = Vector.begin(), End = Vector.end(); j != End; ++j) ; }

See also source code http://www-device.eecs.berkeley.edu/bsim/?page=BSIM4_LR as example of feature application.

Eugene.

llvmbot commented 9 years ago

I see. Contributions are always welcome!

EugeneZelenko commented 9 years ago

Hi, Alexander!

static void Conditional(bool Condition) { int i;

if (Condition) { i = 0; }

// i is not usex in other parts of code }

static void ConditionalFixed(bool Condition) { if (Condition) { int i = 0; }

// i is not used in other parts of code }

static void Loop() { int Variable;

for (int i = 0; i < 10; ++i) { Variable = 0; // variable is reinitialized on each iteration

  // doing something
}

// Variable is not used in other parts of code }

static void LoopFixed() { for (int i = 0; i < 10; ++i) { int Variable = 0;

  // doing something
}

// Variable is not used in other parts of code }

Eugene.

llvmbot commented 9 years ago

Please add an example of code before and after the fix and a more specific description of what (and why) the check should do.