whisperity marked 6 inline comments as done.
whisperity added inline comments.
Herald added a subscriber: nullptr.cpp.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:177
+ const std::size_t StartIndex) {
+ const std::size_t NumParams = FD->getNumParams();
+ assert(StartIndex < NumParams && "out of bounds for start");
----------------
aaron.ballman wrote:
> Some interesting test cases to consider: varargs functions and K&R C functions
> K&R C functions
Call me too young, but I had to look up what a "K&R C function" is, and I am
absolutely baffled how this unholy construct is still supported! **But:**
thanks to Clang supporting it properly in the AST, the checker works out of the
box!
Given
```
int foo(a, b)
int a;
int b;
{
return a + b;
}
```
We get the following output:
```
/tmp/knr.c:2:3: warning: 2 adjacent parameters of 'foo' of similar type ('int')
are easily swapped by mistake [bugprone-easily-swappable-parameters]
int a;
^
/tmp/knr.c:2:7: note: the first parameter in the range is 'a'
int a;
^
/tmp/knr.c:3:7: note: the last parameter in the range is 'b'
int b;
^
```
(even the locations are consistent!)
Should I add a test case for this? We could use a specifically C test case
either way eventually...
-----
> varargs functions
This is a bit of terminology, but something tells me you meant the //variadic
function// here, right? As opposed to type parameter packs.
Given
```
int sum(int ints...) {
return 0;
}
```
the AST looks something like this:
```
`-FunctionDecl 0x56372e29e258 <variadic.cpp:1:1, line:3:1> line:1:5 sum 'int
(int, ...)'
|-ParmVarDecl 0x56372e29e188 <col:9, col:13> col:13 ints 'int'
```
Should we diagnose this? And if so, how? The variadic part is not represented
(at least not at first glance?) in the AST. Understanding the insides of such a
function would require either overapproximatic stuff and doing a looot of extra
handling, or becoming flow sensitive... and we'd still need to understand all
the `va_` standard functions' semantics either way.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69560/new/
https://reviews.llvm.org/D69560
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
