From 2f686697187e8834346b7924797d44c978252ec6 Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Sat, 22 Jun 2013 06:43:58 +0000 Subject: [PATCH] Revert r184401 which reverted r183462. The problem with r183462 was that we assumed that a diagnostic id of zero would be silent. This small correction to CheckDerivedToBaseConversion changes it's behavior to omit the diagnostic when given a diagnostic id of zero. This fix passes the test case added in r184402. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@184631 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticSemaKinds.td | 4 +- lib/Sema/SemaDeclCXX.cpp | 59 +++++++++++++--------- test/CXX/drs/dr0xx.cpp | 9 ++-- 3 files changed, 42 insertions(+), 30 deletions(-) diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 944eca64ba..5d7d2372bd 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -1022,8 +1022,8 @@ def err_access_dtor_base : Error<"base class %0 has %select{private|protected}1 destructor">, AccessControl; def err_access_dtor_vbase : - Error<"inherited virtual base class %0 has " - "%select{private|protected}1 destructor">, + Error<"inherited virtual base class %1 has " + "%select{private|protected}2 destructor">, AccessControl; def err_access_dtor_temp : Error<"temporary of type %0 has %select{private|protected}1 destructor">, diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index cbe4a2829b..caf7affd0c 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -1311,7 +1311,7 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *Class, assert(BaseDecl && "Record type has no declaration"); BaseDecl = BaseDecl->getDefinition(); assert(BaseDecl && "Base type is not incomplete, but has no definition"); - CXXRecordDecl * CXXBaseDecl = cast(BaseDecl); + CXXRecordDecl *CXXBaseDecl = cast(BaseDecl); assert(CXXBaseDecl && "Base type is not a C++ type"); // C++ [class]p3: @@ -1590,26 +1590,28 @@ Sema::CheckDerivedToBaseConversion(QualType Derived, QualType Base, return false; } - // We know that the derived-to-base conversion is ambiguous, and - // we're going to produce a diagnostic. Perform the derived-to-base - // search just one more time to compute all of the possible paths so - // that we can print them out. This is more expensive than any of - // the previous derived-to-base checks we've done, but at this point - // performance isn't as much of an issue. - Paths.clear(); - Paths.setRecordingPaths(true); - bool StillOkay = IsDerivedFrom(Derived, Base, Paths); - assert(StillOkay && "Can only be used with a derived-to-base conversion"); - (void)StillOkay; - - // Build up a textual representation of the ambiguous paths, e.g., - // D -> B -> A, that will be used to illustrate the ambiguous - // conversions in the diagnostic. We only print one of the paths - // to each base class subobject. - std::string PathDisplayStr = getAmbiguousPathsDisplayString(Paths); - - Diag(Loc, AmbigiousBaseConvID) - << Derived << Base << PathDisplayStr << Range << Name; + if (AmbigiousBaseConvID) { + // We know that the derived-to-base conversion is ambiguous, and + // we're going to produce a diagnostic. Perform the derived-to-base + // search just one more time to compute all of the possible paths so + // that we can print them out. This is more expensive than any of + // the previous derived-to-base checks we've done, but at this point + // performance isn't as much of an issue. + Paths.clear(); + Paths.setRecordingPaths(true); + bool StillOkay = IsDerivedFrom(Derived, Base, Paths); + assert(StillOkay && "Can only be used with a derived-to-base conversion"); + (void)StillOkay; + + // Build up a textual representation of the ambiguous paths, e.g., + // D -> B -> A, that will be used to illustrate the ambiguous + // conversions in the diagnostic. We only print one of the paths + // to each base class subobject. + std::string PathDisplayStr = getAmbiguousPathsDisplayString(Paths); + + Diag(Loc, AmbigiousBaseConvID) + << Derived << Base << PathDisplayStr << Range << Name; + } return true; } @@ -3790,10 +3792,17 @@ Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location, CXXDestructorDecl *Dtor = LookupDestructor(BaseClassDecl); assert(Dtor && "No dtor found for BaseClassDecl!"); - CheckDestructorAccess(ClassDecl->getLocation(), Dtor, - PDiag(diag::err_access_dtor_vbase) - << VBase->getType(), - Context.getTypeDeclType(ClassDecl)); + if (CheckDestructorAccess( + ClassDecl->getLocation(), Dtor, + PDiag(diag::err_access_dtor_vbase) + << Context.getTypeDeclType(ClassDecl) << VBase->getType(), + Context.getTypeDeclType(ClassDecl)) == + AR_accessible) { + CheckDerivedToBaseConversion( + Context.getTypeDeclType(ClassDecl), VBase->getType(), + diag::err_access_dtor_vbase, 0, ClassDecl->getLocation(), + SourceRange(), DeclarationName(), 0); + } MarkFunctionReferenced(Location, const_cast(Dtor)); DiagnoseUseOfDecl(Dtor, Location); diff --git a/test/CXX/drs/dr0xx.cpp b/test/CXX/drs/dr0xx.cpp index 7df3d7eb6c..c51e8c82e4 100644 --- a/test/CXX/drs/dr0xx.cpp +++ b/test/CXX/drs/dr0xx.cpp @@ -59,10 +59,13 @@ namespace dr5 { // dr5: yes const C c = e; } -namespace dr7 { // dr7: no +namespace dr7 { // dr7: yes class A { public: ~A(); }; - class B : virtual private A {}; - class C : public B {} c; // FIXME: should be rejected, ~A is inaccessible + class B : virtual private A {}; // expected-note 2 {{declared private here}} + class C : public B {} c; // expected-error 2 {{inherited virtual base class 'dr7::A' has private destructor}} \ + // expected-note {{implicit default constructor for 'dr7::C' first required here}} \ + // expected-note {{implicit destructor for 'dr7::C' first required here}} + class VeryDerivedC : public B, virtual public A {} vdc; class X { ~X(); }; // expected-note {{here}} class Y : X { ~Y() {} }; // expected-error {{private destructor}} -- 2.40.0