Don't instantiate deleted methods.

[GoogleCodeExporter]

include-what-you-use traverses deleted constructors, destructors and methods 
because they aren't defined. This triggers an assert in clang (and all sorts of 
bizarre compilation errors if clang isn't built in debug mode).

For instance, this assert is triggered when we call 
Sema::DefineImplicitDestructor

  assert((Destructor->isDefaulted() &&
          !Destructor->doesThisDeclarationHaveABody() &&
          !Destructor->isDeleted()) &&
         "DefineImplicitDestructor - call it for implicit default dtor");

Original issue reported on code.google.com by SmSpil...@gmail.com on 6 Oct 2014 at 1:11

Attachments:

• dont_traverse_deleted_methods.patch

[GoogleCodeExporter]

Thanks for the patch, much appreciated!

Could you also include a test case, or post some kind of reduced test case that 
needs this patch?

Original comment by kim.gras...@gmail.com on 6 Oct 2014 at 6:51

[GoogleCodeExporter]

I can try and add one to the patch later today.

For now I'd imagine it'd be something like:

class A
{
public:

    A (const &A) = delete;
    A & operator= (const &A) = delete;
};

Original comment by SmSpil...@gmail.com on 7 Oct 2014 at 1:17

[GoogleCodeExporter]

Hi,

I've included two test cases in this revised patch.

Original comment by SmSpil...@gmail.com on 15 Oct 2014 at 4:45

Attachments:

• dont_traverse_deleted_methods.patch

[GoogleCodeExporter]

Thank you!

Here's an updated (Git) patch based on your work, I've made the following 
changes:

- Remove unused CXXDestructorDecl* variable
- Merge test cases into one file
- Add assertion for Clang diagnostics

Let me know if this looks good, and I can commit it!

Original comment by kim.gras...@gmail.com on 23 Oct 2014 at 8:51

Attachments:

• issue159.diff

[GoogleCodeExporter]

Looks good to me and thanks for consolidating and cleaning it up. I'd say 
commit it :)

Original comment by SmSpil...@gmail.com on 24 Oct 2014 at 12:24

[GoogleCodeExporter]

Looks good. The only question is why in test case deleted move constructor is 
after deleted implicitly defined destructor? It would be nice if we had 
constructors and destructors separated. Should we test other deleted methods in 
addition to implicit?

Original comment by vsap...@gmail.com on 24 Oct 2014 at 5:59

[GoogleCodeExporter]

> The only question is why in test case deleted move constructor
> is after deleted implicitly defined destructor?

I don't know, I just added them as they occurred to me. Is there a better order?

> Should we test other deleted methods in addition to implicit?

I did some ad-hoc tests with explicitly deleted methods and they didn't exhibit 
the same symptoms, so I figured it was covered elsewhere or known to work.

Original comment by kim.gras...@gmail.com on 24 Oct 2014 at 6:30

[GoogleCodeExporter]

Oh, now I saw the concluding sentence, that it would be better to keep ctors 
and dtors separate. OK, let me fix that up quickly.

Original comment by kim.gras...@gmail.com on 24 Oct 2014 at 6:31

[GoogleCodeExporter]

Please take another look.

Original comment by kim.gras...@gmail.com on 24 Oct 2014 at 6:34

Attachments:

• issue159-2.diff

[GoogleCodeExporter]

Everything looks good to me.

Original comment by vsap...@gmail.com on 24 Oct 2014 at 4:26

[GoogleCodeExporter]

Fixed in r589, thanks for the patch!

Original comment by kim.gras...@gmail.com on 24 Oct 2014 at 4:54

• Changed state: Fixed