[PATCH] D61816: [CFG] [analyzer] pr41300: Add a branch to skip virtual base initializers when they are handled by the superclass.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Hmmm, how about this?

  #include <iostream>
  
  struct VBase1 {
    VBase1() { std::cout << "VBase1()\n"; }
    VBase1(int) { std::cout << "VBase1(int)\n"; }
  };
  
  struct VBase2 {
    VBase2() { std::cout << "VBase2()\n"; }
    VBase2(std::string) { std::cout << "VBase2(std::string)\n"; }
  };
  
  struct B : public virtual VBase1 {
    B() : VBase1(0) {}
  };
  
  struct C : public B, public virtual VBase2 {
    C() : VBase2("sajt") {}
  };
  
  int main() {
    C c;
  }

Output:

  VBase1()
  VBase2(std::string)

You are right that all virtual bases as initialized before everything else, but not all ctor delegations are skipped. If not this specific one, can something similar screw us over?



================
Comment at: clang/lib/Analysis/CFG.cpp:1434-1438
+  // For C++ constructor add initializers to CFG. Virtual base classes may have
+  // already been initialized by the superclass. Make a branch so that it was
+  // possible to jump straight to non-virtual bases and fields, skipping
+  // virtual bases. We only need to do this once because all virtual base
+  // initializers go all together before all other initializers.
----------------
I find this a little too vague. The standard states:

> First, and only for the constructor of the most derived class (4.5) , virtual base classes are initialized in the order they appear on a depth-first left-to-right traversal of the directed acyclic graph of base classes, where “left-to-right” is the order of appearance of the base classes in the derived class base-specifier-list.
>
> Then, direct base classes are initialized in declaration order as they appear in the (regardless of the order of the mem-initializer s).
>
> Then, non-static data members are initialized in the order they were declared in the class definition (again regardless of the order of the mem-initializer s).
>
> Finally, the base-specifier-list compound-statement of the constructor body is executed.

This would be an overkill here, but how does this sound like:

> Constructor delegations are ignored to virtual bases unless the object is of the most derived class.
>
>  class VBase { VBase() = default; VBase(int) {} };
>  class A : virtual public VBase { A() : VBase(0) {} };
>  class B : public A {};
>
>  B b; // Constructor calls in order: VBase(), A(), B().
>       // VBase(int) is ignored, B isn't the most derived class that
>       // delegates to the virtual base.
>
> This may result in the virtual base(s) being already initialized at this point, in which case we should jump right onto non-virtual bases and fields. To handle this, make a CFG branch. We only need to do this once, since the standard states that all virtual bases shall be initialized before non-virtual bases and direct data members.

Also, the comment of mine complementing this inline raises a concern about "doing this only once", can you specify?


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

https://reviews.llvm.org/D61816