[PATCH] D26195: Ignore incomplete types when determining whether they are expensive to copy

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D26195#584958, @flx wrote:

> In https://reviews.llvm.org/D26195#584730, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D26195#584724, @flx wrote:
> >
> > > In https://reviews.llvm.org/D26195#584712, @aaron.ballman wrote:
> > >
> > > > Please add a test case with an incomplete type that would exercise this code path, otherwise, LGTM.
> > >
> > >
> > > Hi Aaron,
> > >
> > > do you have any advise on how to add an incomplete type? When debugging this I had a compilation unit that failed to compile causing it, but I'm not sure this is a good way to add a test case.
> >
> >
> > A type like `class C;` is an incomplete type, as is `void`, so perhaps you can find a check that would let such a construct call through to `isExpensiveToCopy()`.
>
>
> Great, this works and I was able to see the check produce a false positive without the proposed change here, but the test code introduces a compile error now due to the incomplete type used in the function definition. Is there a way to suppress that?


Unlikely -- fixing the compile error likely makes the type not expensive to copy by using a pointer (or reference). This may be tricky to test because the times when you would call `isExpensiveToCopy()` is with types that are going to be logically required to be complete. I am not certain the compile error is actually a problem though -- I would imagine your existing false-positives (that you mentioned in the patch summary) are cases where there is a compile error *and* a clang-tidy diagnostic, so the test may simply be "check that there's only a compile error and no clang-tidy diagnostic where there used to be a false-positive one."


Repository:
  rL LLVM

https://reviews.llvm.org/D26195