r183462 - Implement DR7

[Richard Smith]

On Thu, Jun 6, 2013 at 4:43 PM, David Majnemer <david.majnemer at gmail.com> wrote:
> Author: majnemer
> Date: Thu Jun  6 18:43:20 2013
> New Revision: 183462
>
> URL: http://llvm.org/viewvc/llvm-project?rev=183462&view=rev
> Log:
> Implement DR7
>
> Disallowing deriving from classes that have private virtual base classes
> except in instances where the deriving class would be able to cast
> itself to the private virtual base via a different derivation.
>
> Modified:
>     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>     cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>     cfe/trunk/test/CXX/drs/dr0xx.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=183462&r1=183461&r2=183462&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Jun  6 18:43:20 2013
> @@ -1011,8 +1011,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">,
>
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=183462&r1=183461&r2=183462&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Jun  6 18:43:20 2013
> @@ -1311,7 +1311,7 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *
>    assert(BaseDecl && "Record type has no declaration");
>    BaseDecl = BaseDecl->getDefinition();
>    assert(BaseDecl && "Base type is not incomplete, but has no definition");
> -  CXXRecordDecl * CXXBaseDecl = cast<CXXRecordDecl>(BaseDecl);
> +  CXXRecordDecl *CXXBaseDecl = cast<CXXRecordDecl>(BaseDecl);
>    assert(CXXBaseDecl && "Base type is not a C++ type");
>
>    // C++ [class]p3:
> @@ -3806,10 +3806,17 @@ Sema::MarkBaseAndMemberDestructorsRefere
>
>      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(),

The 0 here is the cause of the regression. Diagnostic 0 just happens
to be diag::err_attribute_not_type_attr, and
CheckDerivedToBaseConversion doesn't check whether it's 0 before
emitting it.

We should probably reserve diagnostic ID 0 and assert if anyone tries
to emit it, because we use it to mean "no diagnostic" in several
places already...

> +          SourceRange(), DeclarationName(), 0);
> +    }
>
>      MarkFunctionReferenced(Location, const_cast<CXXDestructorDecl*>(Dtor));
>      DiagnoseUseOfDecl(Dtor, Location);
>
> Modified: cfe/trunk/test/CXX/drs/dr0xx.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/drs/dr0xx.cpp?rev=183462&r1=183461&r2=183462&view=diff
> ==============================================================================
> --- cfe/trunk/test/CXX/drs/dr0xx.cpp (original)
> +++ cfe/trunk/test/CXX/drs/dr0xx.cpp Thu Jun  6 18:43:20 2013
> @@ -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 default 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}}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits