[PATCH] D30131: [profiling] PR31992: Don't skip interesting non-base constructors

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

In https://reviews.llvm.org/D30131#684310, @arphaman wrote:

> LGTM.
>
> One point to note, when we are displaying coverage for constructors that have both the base and complete versions instrumented, e.g.:
>
>   class Foo {
>   public:
>       Foo() { }
>   };
>  
>   class Bar : virtual public Foo {
>   public:
>      Bar() { }; // llvm-cov will show 2 instantiations for Bar()
>   };
>  
>   class BarBar: public Bar {
>   public:
>     BarBar() { }
>   };
>  
>   int main (int argc, char* argv[])
>   {
>       BarBar b;
>       Bar bb;
>   }
>
>
> llvm-cov will treat each constructor as an instantiation (like a template instantiation). Should they be treated as instantiations though?


No, ideally we'd combine the coverage reporting for Bar's constructors, since it's unlikely that users care about the distinction between them:

  /tmp/x.cc:
      1|       |class Foo {
      2|       |public:
      3|      2|    Foo() { }
      4|       |};
      5|       |
      6|       |class Bar : virtual public Foo {
      7|       |public:
      8|      2|   Bar() { }; // llvm-cov will show 2 instantiations for Bar()
    ------------------
    | _ZN3BarC2Ev:        //< It doesn't help that these symbols demangle to the same name..
    |    8|      1|   Bar() { }; // llvm-cov will show 2 instantiations for Bar()
    ------------------
    | _ZN3BarC1Ev:
    |    8|      1|   Bar() { }; // llvm-cov will show 2 instantiations for Bar()
    ------------------
      9|       |};
     10|       |
     11|       |class BarBar: public Bar {
     12|       |public:
     13|      1|  BarBar() { }
     14|       |};
     15|       |
     16|       |int main (int argc, char* argv[])
     17|      1|{
     18|      1|    BarBar b; // calls _ZN6BarBarC1Ev -> {_ZN3FooC2Ev, _ZN3BarC2Ev} (leaves)
     19|      1|    Bar bb; // calls _ZN3BarC1Ev -> _ZN3FooC2Ev (leaf)
     20|      1|}

I don't think this report is 'terrible' though. It's certainly better than saying "BarBar"/"Bar" are executed 0/1 times, which is the status quo.


https://reviews.llvm.org/D30131