On Fri, Mar 6, 2015 at 9:57 AM, Nico Weber <[email protected]> wrote:
> On Fri, Mar 6, 2015 at 9:32 AM, David Blaikie <[email protected]> wrote: > >> >> >> On Thu, Mar 5, 2015 at 10:01 PM, Nico Weber <[email protected]> wrote: >> >>> Author: nico >>> Date: Fri Mar 6 00:01:06 2015 >>> New Revision: 231451 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=231451&view=rev >>> Log: >>> Don't crash on non-public referenced dtors in toplevel classes. >>> >>> Fixes PR22793, a bug that caused self-hosting to fail after the innocuous >>> r231254. See the bug for details. >>> >> >> Awesome - thanks for jumping on this! >> >> >>> >>> Modified: >>> cfe/trunk/lib/Sema/SemaExpr.cpp >>> cfe/trunk/test/CodeGenCXX/trivial-constructor-init.cpp >>> >>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=231451&r1=231450&r2=231451&view=diff >>> >>> ============================================================================== >>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) >>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Mar 6 00:01:06 2015 >>> @@ -117,7 +117,7 @@ static AvailabilityResult DiagnoseAvaila >>> case AR_Available: >>> case AR_NotYetIntroduced: >>> break; >>> - >>> + >>> case AR_Deprecated: >>> if (S.getCurContextAvailability() != AR_Deprecated) >>> S.EmitAvailabilityWarning(Sema::AD_Deprecation, >>> @@ -11859,8 +11859,11 @@ void Sema::MarkFunctionReferenced(Source >>> } else if (CXXDestructorDecl *Destructor = >>> dyn_cast<CXXDestructorDecl>(Func)) { >>> Destructor = cast<CXXDestructorDecl>(Destructor->getFirstDecl()); >>> - if (Destructor->isDefaulted() && !Destructor->isDeleted()) >>> + if (Destructor->isDefaulted() && !Destructor->isDeleted()) { >>> + if (Destructor->isTrivial() && >>> !Destructor->hasAttr<DLLExportAttr>()) >>> + return; >>> DefineImplicitDestructor(Loc, Destructor); >>> + } >>> if (Destructor->isVirtual() && getLangOpts().AppleKext) >>> MarkVTableUsed(Loc, Destructor->getParent()); >>> } else if (CXXMethodDecl *MethodDecl = dyn_cast<CXXMethodDecl>(Func)) >>> { >>> >>> Modified: cfe/trunk/test/CodeGenCXX/trivial-constructor-init.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/trivial-constructor-init.cpp?rev=231451&r1=231450&r2=231451&view=diff >>> >>> ============================================================================== >>> --- cfe/trunk/test/CodeGenCXX/trivial-constructor-init.cpp (original) >>> +++ cfe/trunk/test/CodeGenCXX/trivial-constructor-init.cpp Fri Mar 6 >>> 00:01:06 2015 >>> @@ -32,3 +32,17 @@ static C c[4]; >>> >>> int main() { >>> } >>> + >>> +namespace PR22793 { >>> +template <typename> >>> +struct foo { >>> +protected: >>> >> >> 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? >> > > 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. > Ah, I see - just an optimization, then. Thanks for the context. > 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, > Maybe a detail missed from the commit message? I don't see that detail mentioned. > constructors have the same check a few lines further up. > Ah, looking at the code I see - fair enough then. Thanks for the details! - David
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
