https://github.com/PiotrZSL commented:

Few comments:
First I attach part of a check that do same thing, you can take a look into it: 
[sample.txt](https://github.com/user-attachments/files/24554936/sample.txt)

Second based on my experience in implementing such check:
- Do not implement this with usage of things like "getParents" - it will be to 
slow, write RecursiveASTVisitor, in such case it will be easy to find out about 
scope when entering lambdas, loops, exception handlers, in such case also it 
will be easier to check all scope levels without need to build list of usage, 
all you will need will be just to track variables.
- Next in case of C it's easier, but implementing such check for C alone is a 
waste. You need to take into account calls to functions with side effects 
during variable initialization, in such case having 2 configuration options to 
include/exclude them would be nice
- In case of CPP you need also configuration options to exclude some Types 
(RAII objects) and handling calls to members and operators.
- At the last you may consider having option to exclude some types, like 
builtin and const variables, as those most o f the time will be optimized out.
- Diagnostic should also point to a new preferred scope for variable definition
- As for category of the checks, if it's C only, then readability, if it's C++ 
then performance could also be fine. Name should also be more detailed. Exclude 
functions from this check. Focus only on variables.
- Current code looks too complicated
- Good idea with loop variables.
- Consider excluding implicit templates.
- You may need consider issues with examples like this:
```
int var2 = 20;
int var = var2;
var2++;
if (var3 > 100)
{
    // var cannot be moved here because it would hold different value, same 
issues with side-effect functions that modify var2 and with std::move.
     return var;
}
```
You may see that check require lot of configuration options to enable ability 
to deal with false-positives.


https://github.com/llvm/llvm-project/pull/175429
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to