<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Mar 6, 2015 at 10:04 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><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 dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Fri, Mar 6, 2015 at 9:57 AM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br><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 dir="ltr"><div><div><div class="gmail_extra"><div class="gmail_quote">On Fri, Mar 6, 2015 at 9:32 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><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 dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Thu, Mar 5, 2015 at 10:01 PM, Nico Weber <span dir="ltr"><<a href="mailto:nicolasweber@gmx.de" target="_blank">nicolasweber@gmx.de</a>></span> wrote:<br><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">Author: nico<br>
Date: Fri Mar 6 00:01:06 2015<br>
New Revision: 231451<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=231451&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=231451&view=rev</a><br>
Log:<br>
Don't crash on non-public referenced dtors in toplevel classes.<br>
<br>
Fixes PR22793, a bug that caused self-hosting to fail after the innocuous<br>
r231254. See the bug for details.<br></blockquote></span><div><br>Awesome - thanks for jumping on this! <br> </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">
<br>
Modified:<br>
cfe/trunk/lib/Sema/SemaExpr.cpp<br>
cfe/trunk/test/CodeGenCXX/trivial-constructor-init.cpp<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=231451&r1=231450&r2=231451&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=231451&r1=231450&r2=231451&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Mar 6 00:01:06 2015<br>
@@ -117,7 +117,7 @@ static AvailabilityResult DiagnoseAvaila<br>
case AR_Available:<br>
case AR_NotYetIntroduced:<br>
break;<br>
-<br>
+<br>
case AR_Deprecated:<br>
if (S.getCurContextAvailability() != AR_Deprecated)<br>
S.EmitAvailabilityWarning(Sema::AD_Deprecation,<br>
@@ -11859,8 +11859,11 @@ void Sema::MarkFunctionReferenced(Source<br>
} else if (CXXDestructorDecl *Destructor =<br>
dyn_cast<CXXDestructorDecl>(Func)) {<br>
Destructor = cast<CXXDestructorDecl>(Destructor->getFirstDecl());<br>
- if (Destructor->isDefaulted() && !Destructor->isDeleted())<br>
+ if (Destructor->isDefaulted() && !Destructor->isDeleted()) {<br>
+ if (Destructor->isTrivial() && !Destructor->hasAttr<DLLExportAttr>())<br>
+ return;<br>
DefineImplicitDestructor(Loc, Destructor);<br>
+ }<br>
if (Destructor->isVirtual() && getLangOpts().AppleKext)<br>
MarkVTableUsed(Loc, Destructor->getParent());<br>
} else if (CXXMethodDecl *MethodDecl = dyn_cast<CXXMethodDecl>(Func)) {<br>
<br>
Modified: cfe/trunk/test/CodeGenCXX/trivial-constructor-init.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/trivial-constructor-init.cpp?rev=231451&r1=231450&r2=231451&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/trivial-constructor-init.cpp?rev=231451&r1=231450&r2=231451&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/CodeGenCXX/trivial-constructor-init.cpp (original)<br>
+++ cfe/trunk/test/CodeGenCXX/trivial-constructor-init.cpp Fri Mar 6 00:01:06 2015<br>
@@ -32,3 +32,17 @@ static C c[4];<br>
<br>
int main() {<br>
}<br>
+<br>
+namespace PR22793 {<br>
+template <typename><br>
+struct foo {<br>
+protected:<br></blockquote></div></div><div><br>Naively, I was hoping a/the fix might resolve around this being protected. I was sort of hoping that the fix might involve /removing/ code that was needlessly checking access rather than adding more conditions. Seems strange that the access level could change the behavior of this code. (but I haven't fully understood the notes you made on the bug - so I imagine it's probably described in there somewhere) Any chance of that?</div></div></div></div></blockquote></div><br></div></div></div><div class="gmail_extra">Public defaulted dtors are marked as "irrelevant destructors", and that triggers an early return somewhere. If a destructor is not public, it isn't irrelevant to sema though, as it now needs access checks.</div></div></blockquote></div></div><div><br>Ah, I see - just an optimization, then. Thanks for the context.<br> </div><span class=""><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 dir="ltr"><div class="gmail_extra"> So the early return that "protects" irrelevant destructors doesn't fire for this case, and we need to explicitly check for triviality. As the commit message says,</div></div></blockquote></span><div><br>Maybe a detail missed from the commit message? I don't see that detail mentioned.<br></div></div></div></div></blockquote><div><br></div><div>Ah sorry, it was on the bug, not in the commit message: <a href="http://llvm.org/bugs/show_bug.cgi?id=22793#c5">http://llvm.org/bugs/show_bug.cgi?id=22793#c5</a></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 dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><span class=""><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 dir="ltr"><div class="gmail_extra"> constructors have the same check a few lines further up.</div></div></blockquote></span><div><br>Ah, looking at the code I see - fair enough then.</div><div><br>Thanks for the details!<span class=""><font color="#888888"><br><br>- David </font></span></div></div><br></div></div>
</blockquote></div><br></div></div>