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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to