[PATCH] D69308: [analyzer] Test cases for the unsupported features for Clang Static Analyzer

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Thanks for the tests!

Both of these features are relatively hard.

Operator `new[]` requires invoking multiple (potentially unknown) amount of constructors with the same construct-expression. Apart from the technical difficulties of juggling program points around correctly to avoid accidentally merging paths together, you'll have to be a judge on when to exit the loop and how to widen it. Given that the constructor is going to be a default constructor, a nice 95% solution might be to execute exactly one constructor and then default-bind the resulting `LazyCompoundVal` to the whole array; it'll work whenever the default constructor doesn't touch global state but only initializes the object to various default values. But if, say, you're making an array of strings, depending on the implementation you might have to allocate a new buffer for each string, and in this case default-binding won't cut it. You might want to come up with an auxiliary analysis in order to perform widening of these simple loops more precisely.

Default arguments are annoying because the initializer expression is evaluated at the call site but doesn't syntactically belong to the caller's AST; instead it belongs to the `ParmVarDecl` for the default parameter. This can lead to situations when the same expression has to carry different values simultaneously - when multiple instances of the same function are evaluated as part of the same full-expression without specifying the default arguments. Even simply calling the function twice (not necessarily within the same full-expression) may lead to program points agglutinating because it's the same expression. There are some nasty test cases already in `temporaries.cpp` (`struct DefaultParam` and so on). I recommend adding a new `LocationContext` kind specifically to deal with this problem. It'll also help you figure out the construction context when you evaluate the construct-expression (though you might still need to do some additional CFG work to get construction contexts right).



================
Comment at: clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_default_arguments.cpp:3-6
+// REQUIRES: non-existing-system
+
+// These test cases demonstrate lack of Static Analyzer features.
+// Remove REQUIRES line, when the feature gets implemented.
----------------
I prefer our FIXME-tests to keep running while documenting incorrect results, so that people were informed when they fix something. Eg.:

```lang=c++
// FIXME: Should be TRUE.
clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
```

I doubt that the whole file of tests will be fixed by a single commit, so they'll have to change this anyway.

It's also nice to inform people that they accidentally changed something they didn't expect to change.


================
Comment at: clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_default_arguments.cpp:58
+
+int called_h(init_default_member l = init_default_member()) {
+  //We expect that the analyzer assumes the default value (call site test3)
----------------
Note that default arguments may also be of reference type, eg.:
```lang=c++
void foo(const X &x = X(1, 2, 3));
void bar(Y &&y = Y(4, 5, 6));
```


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69308/new/

https://reviews.llvm.org/D69308