[PATCH] D63920: [CTU] Add support for virtual functions

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D63920#1568049 <https://reviews.llvm.org/D63920#1568049>, @Szelethus wrote:

> In D63920#1568031 <https://reviews.llvm.org/D63920#1568031>, @martong wrote:
>
> > In D63920#1566035 <https://reviews.llvm.org/D63920#1566035>, @baloghadamsoftware wrote:
> >
> > > Try to set analyzer option `IPAMode` to something different from its default value which is `dynamic-bifurcate` in the test file.
> >
> >
> > Ok I set it to `ipa=inlining` and then we receive the expected "UNKNOWN" warning.
>
>
> Alright, what are testing here exactly? I think its fine to see how the new virtual functions are inlined in different inlining mode, but we should definitely have a `RUN:` line/test file with the default mode.


So, we would like to have a test which indicates that we can inline a virtual function from another TU. These tests are at line 110 and 111.

At line 113 we would like to have a test which verifies that if the dynamic type is unknown there is not much we can gain with inlining.
So, either we use the default ipa mode and then we will have

  clang_analyzer_eval(obj->fvcl(1));           // expected-warning{{TRUE}} expected-warning{{UNKOWN}}

Or we use` ipa=inlining` and then we have

  clang_analyzer_eval(obj->fvcl(1));           // expected-warning{{UNKOWN}}

The latter seems to be more meaningful for me because it is hard to explain why we expect {{TRUE}} in the first case.
Still I vote for a third alternative when we use the default ipa mode:

  clang_analyzer_eval(obj->fvcl(1) == 8);           // expected-warning{{TRUE}} expected-warning{{FALSE}}

This seems to be clear for me , it is obvious from the warnings that the dynamic type is unknown.

Or perhaps there is no need to have a check for the case when the dynamic type is unknown?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63920