<div dir="ltr">On Fri, Dec 20, 2013 at 4:18 PM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div class="im">On Dec 20, 2013, at 4:06 PM, Reid Kleckner <<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>> wrote:<br>
</div><div><div class="im"><blockquote type="cite"><div dir="ltr">+John, since it was his code.</div></blockquote><blockquote type="cite"><div dir="ltr"><div><br></div><div>Are there other ways we can get an invalid record decl and then do layout on a derived class to trigger <span style="font-family:arial,sans-serif;font-size:13px">PR18284?  If so, it seems like this doesn't fix the underlying issue.</span></div>
</div></blockquote><div><br></div></div>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.</div>
</div></blockquote><div><br></div><div>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:</div>
<div><br><div>  class A {};</div><div>  class C : public A {};</div><div>  void A::RunTest() {}<br></div></div><div><br></div><div>It seems like whatever caused the crash that's fixed by r110891 is now also fixed in some other way.</div>
<div><br></div><div>This is the example where clang currently marks A as invalid but keeps C valid:<br></div><div><br></div><div><div>  class A {};</div><div>  class C : public A {};</div><div>  A::RunTest() {}</div></div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><br>
</div><div>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?</div></div></blockquote><div><br></div><div>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).</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><span class=""><font color="#888888"><div>
<br></div><div>John.</div></font></span></div></blockquote></div><br></div></div>