[PATCH] D19769: [clang-tidy] Add explicitly given array size heuristic to misc-suspicious-missing-comma check.

[Dominik Szabó via cfe-commits]

szdominik added a comment.

The original size is available - but from the decleration, not from the initListExpr.


================
Comment at: clang-tidy/misc/SuspiciousMissingCommaCheck.cpp:106
@@ +105,3 @@
+  if (InitializerList->hasArrayFiller()) {
+      diag(InitializerList->getExprLoc(),
+           "wrong string array initialization: "
----------------
etienneb wrote:
> szdominik wrote:
> > etienneb wrote:
> > > The error should still be reported to the missing comma (concatenated token):
> > >   ConcatenatedLiteral->getLocStart(),
> > > 
> > > We could add a NOTE to point to the array, stating that the size mismatch.
> > > 
> > > What do you think?
> > Interesting question (the first idea was that we can't decide that the comma is missing or the size is wrong, so report to the array, that's a more secure solution), but I agree that the note could be more effective.
> > And... it's still a suspicious-missing-comma checker, not a wrong-string-array-size checker :)
> How can you be sure the size was provided by the user? And not inferred by the type system?
> 
> For the following examples:
> ```
> const char* listA[] = {"a", "b" };
> const char* listB[5] = {"a", "b" };
> ```
> 
> We've got this:
> ```
> VarDecl 0x5e9d840 <C:\src\llvm\examples\hello_world.cc:11:1, col:33> col:13 listA 'const char *[2]' cinit
> `-InitListExpr 0x5e9d950 <col:23, col:33> 'const char *[2]'
>   |-ImplicitCastExpr 0x5e9d978 <col:24> 'const char *' <ArrayToPointerDecay>
>   | `-StringLiteral 0x5e9d8d8 <col:24> 'const char [2]' lvalue "a"
>   `-ImplicitCastExpr 0x5e9d988 <col:29> 'const char *' <ArrayToPointerDecay>
>     `-StringLiteral 0x5e9d8fc <col:29> 'const char [2]' lvalue "b"
> ```
> 
> ```
> VarDecl 0x5e9d840 <C:\src\llvm\examples\hello_world.cc:11:1, col:33> col:13 listA 'const char *[2]' cinit
> `-InitListExpr 0x5e9d950 <col:23, col:33> 'const char *[2]'
>   |-ImplicitCastExpr 0x5e9d978 <col:24> 'const char *' <ArrayToPointerDecay>
>   | `-StringLiteral 0x5e9d8d8 <col:24> 'const char [2]' lvalue "a"
>   `-ImplicitCastExpr 0x5e9d988 <col:29> 'const char *' <ArrayToPointerDecay>
>     `-StringLiteral 0x5e9d8fc <col:29> 'const char [2]' lvalue "b"
> ```
> 
> How can I tell the "size" was written by the user?
> How can you get the "5" and not the "2".
> 
(You copied twice the AST-tree part of listA :))

The array, which was declared with explicit given size, has different type than the usual one. And I can filter the proper cases because of the different (incomplete / constant) array type. 
e.g.

```
VariableDeclaration->getTypeSourceInfo()->getType()->dump()

```
```
const char *string_array_init[3] = {
		"first elem",
		"second elem"
		"third elem"
	};
```
```
ConstantArrayType 0x1efae70 'const char *[3]' 3 
`-PointerType 0x1efae40 'const char *'
  `-QualType 0x1efa511 'const char' const
    `-BuiltinType 0x1efa510 'char'
```
But
```
const char *string_array_init2[] = {
		"first elem",
		"second elem"
		"third elem"
	};
```
```
IncompleteArrayType 0x1f49ba0 'const char *[]' 
`-PointerType 0x1efae40 'const char *'
  `-QualType 0x1efa511 'const char' const
    `-BuiltinType 0x1efa510 'char'
```
However, the type of InitListExpr is always ConstantArrayType (I tried to find a way to be sure the size is explicit given or inferred, but... I don't find anything).
So (unless you haven't better idea) I have two options, as I see:
a) use the array filler solution, but I can't reduce the false-positive cases.
b) use the decleration's type, but I have to use some kind of heuristic, what you told me at your first comment.
Probably we should decide which one is less used.


http://reviews.llvm.org/D19769