[patch] Don't let invalid methods mark their RecordDecl invalid

[Nico Weber]

On Fri, Dec 20, 2013 at 4:18 PM, John McCall <rjmccall at apple.com> wrote:

> On Dec 20, 2013, at 4:06 PM, Reid Kleckner <rnk at google.com> wrote:
>
> +John, since it was his code.
>
>
> Are there other ways we can get an invalid record decl and then do layout
> on a derived class to trigger PR18284?  If so, it seems like this doesn't
> fix the underlying issue.
>
>
> A class should definitely be invalid if any of its base classes is
> invalid.  Making it invalid when one of its members is invalid is probably
> for the best; I would guess that that’s now occurring through some other
> channel, causing the original test to successfully not crash.
>

As far as I can tell, classes aren't always marked invalid if they have
invalid methods at the moment. One example from PR18284 where RunTest is an
invalid method but both B and A are valid:

  class A {};
  class C : public A {};
  void A::RunTest() {}

It seems like whatever caused the crash that's fixed by r110891 is now also
fixed in some other way.

This is the example where clang currently marks A as invalid but keeps C
valid:

  class A {};
  class C : public A {};
  A::RunTest() {}


> I’m not sure what about the linked patch is making a base class invalid
> when a method in a derived class is invalidated, though?
>

No, the patch changes things to not mark a class invalid if one of its
methods is invalid. This was the only place I could find where a class was
marked invalid after its definition is complete. (Hence with this patch,
C's superclass A will be valid too, only its method stays invalid, like
with the first snippet).


>
> John.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131220/7648e4ef/attachment.html>