[PATCH] D47671: [analyzer] Implement copy elision.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In https://reviews.llvm.org/D47671#1122994, @george.karpenkov wrote:

> From my understanding, that's just how c++17 is defined, and that's unrelated to what compiler implementation is used.


Yeah, but this patch is specifically about supporting the pre-C++17 AST, where copy elision was already a well-defined concept, but it wasn't mandatory.

In C++17 copy-constructors are simply omitted from the AST (but `CXXBindTemporaryExpr` might still be accidentally present, and additionally more different construction syntax patterns may appear, eg. we may elide all the way through ?: operators). Support for C++17 was added in the previous patches.

This patch addresses pre-C++17 AST that does include copy-constructors, but they are marked as "(elidable)". They are here because whether to elide them or not was implementation-defined, and even if the elided constructor had arbitrary side effects the compiler was still allowed to choose not to elide it. In this case we decided not to skip in in CFG, but only skip it in the analyzer, because that'd allow the Environment to work with the AST in a straightforward and intuitive manner as it always did. Hence the patch.

----

So there are two reasons why to add an option:

- To be able to disable the new feature as a workaround to a bug in this feature.
- To be able to actually maintain two different behaviors forever as an opt-in feature.

I'm totally fine with the former.

I'm not sure if it'll be easy and worthwile to try doing the latter. We might as well try and see if it will be easy to maintain. I hope it'll mostly work because most syntax patterns with elidable constructors will also appear anyway with non-elidable constructors, albeit much more rarely.

I guess i'll add the flag and see how it goes.


Repository:
  rC Clang

https://reviews.llvm.org/D47671