[PATCH] D74735: [analyzer] Add support for CXXInheritedCtorInitExpr.

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In D74735#1880390 <https://reviews.llvm.org/D74735#1880390>, @xazax.hun wrote:

> If the AST is hard to work with would it make sense to try to change the AST a bit?


Hmm, you mean making `CXXConstructExpr` an abstract base with subclasses `CXXSimpleConstructExpr` and `CXXInheritedConstructExpr`? For me it seems reasonable but I am afraid it would be a huge impact to clang, and not only to the analyzer.



================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:896
+public:
+  virtual const CXXInheritedCtorInitExpr *getOriginExpr() const {
+    return cast<CXXInheritedCtorInitExpr>(AnyFunctionCall::getOriginExpr());
----------------
I wonder whether we could reduce code-duplication using some kind of mixin-like inheritance here.


================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:436
   case CXXConstructExpr::CK_Complete: {
+    assert(CE && "Complete constructors cannot be inherited!");
     std::tie(State, Target) =
----------------
martong wrote:
> Should there be rather `CIE` in the assert? Or the text after `&&` is confusing. 
Surely not `CIE`, since the code below uses `CE`. I see nothing confusing here: `CE` must exist because complete constructors cannot be inherited, thus `CIE` cannot exist.


================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:494
   if (State != Pred->getState()) {
+    assert(CE && "Inherited constructors do not have construction contexts!");
     static SimpleProgramPointTag T("ExprEngine",
----------------
martong wrote:
> `CIE` ?
No. `CE`. Since inherited constructors do not have construction contexts, `State` is the same for `CIE` as the previous `State`. Thus if they are different, we are facing a `CE`.


================
Comment at: clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp:30
+
+namespace arguments_with_constructors {
+struct S {
----------------
Not `constructors_with_arguments`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D74735