Prtially reverted the commit in r338255 to fix a very frequent crash. The code seemed obviously wrong there (calling accessibility checking, passing a possibly unrelated class) and the tests didn't break after reverting it.
Let me know if I missed anything. On Thu, Jul 19, 2018 at 3:37 PM Eric Liu via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: ioeric > Date: Thu Jul 19 06:32:00 2018 > New Revision: 337453 > > URL: http://llvm.org/viewvc/llvm-project?rev=337453&view=rev > Log: > [CodeComplete] Fix accessibilty of protected members from base class. > > Summary: > Currently, protected members from base classes are marked as > inaccessible when completing in derived class. This patch fixes the > problem by > setting the naming class correctly when looking up results in base class > according to [11.2.p5]. > > Reviewers: aaron.ballman, sammccall, rsmith > > Reviewed By: aaron.ballman > > Subscribers: cfe-commits > > Differential Revision: https://reviews.llvm.org/D49421 > > Modified: > cfe/trunk/lib/Sema/SemaAccess.cpp > cfe/trunk/lib/Sema/SemaCodeComplete.cpp > cfe/trunk/test/Index/complete-access-checks.cpp > > Modified: cfe/trunk/lib/Sema/SemaAccess.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaAccess.cpp?rev=337453&r1=337452&r2=337453&view=diff > > ============================================================================== > --- cfe/trunk/lib/Sema/SemaAccess.cpp (original) > +++ cfe/trunk/lib/Sema/SemaAccess.cpp Thu Jul 19 06:32:00 2018 > @@ -11,6 +11,7 @@ > // > > > //===----------------------------------------------------------------------===// > > +#include "clang/Basic/Specifiers.h" > #include "clang/Sema/SemaInternal.h" > #include "clang/AST/ASTContext.h" > #include "clang/AST/CXXInheritance.h" > @@ -1856,29 +1857,31 @@ void Sema::CheckLookupAccess(const Looku > } > } > > -/// Checks access to Decl from the given class. The check will take access > +/// Checks access to Target from the given class. The check will take > access > /// specifiers into account, but no member access expressions and such. > /// > -/// \param Decl the declaration to check if it can be accessed > +/// \param Target the declaration to check if it can be accessed > /// \param Ctx the class/context from which to start the search > -/// \return true if the Decl is accessible from the Class, false > otherwise. > -bool Sema::IsSimplyAccessible(NamedDecl *Decl, DeclContext *Ctx) { > +/// \return true if the Target is accessible from the Class, false > otherwise. > +bool Sema::IsSimplyAccessible(NamedDecl *Target, DeclContext *Ctx) { > if (CXXRecordDecl *Class = dyn_cast<CXXRecordDecl>(Ctx)) { > - if (!Decl->isCXXClassMember()) > + if (!Target->isCXXClassMember()) > return true; > > + if (Target->getAccess() == AS_public) > + return true; > QualType qType = Class->getTypeForDecl()->getCanonicalTypeInternal(); > + // The unprivileged access is AS_none as we don't know how the member > was > + // accessed, which is described by the access in DeclAccessPair. > + // `IsAccessible` will examine the actual access of Target (i.e. > + // Decl->getAccess()) when calculating the access. > AccessTarget Entity(Context, AccessedEntity::Member, Class, > - DeclAccessPair::make(Decl, Decl->getAccess()), > - qType); > - if (Entity.getAccess() == AS_public) > - return true; > - > + DeclAccessPair::make(Target, AS_none), qType); > EffectiveContext EC(CurContext); > return ::IsAccessible(*this, EC, Entity) != ::AR_inaccessible; > } > - > - if (ObjCIvarDecl *Ivar = dyn_cast<ObjCIvarDecl>(Decl)) { > + > + if (ObjCIvarDecl *Ivar = dyn_cast<ObjCIvarDecl>(Target)) { > // @public and @package ivars are always accessible. > if (Ivar->getCanonicalAccessControl() == ObjCIvarDecl::Public || > Ivar->getCanonicalAccessControl() == ObjCIvarDecl::Package) > > Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=337453&r1=337452&r2=337453&view=diff > > ============================================================================== > --- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original) > +++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Thu Jul 19 06:32:00 2018 > @@ -1303,8 +1303,33 @@ namespace { > void FoundDecl(NamedDecl *ND, NamedDecl *Hiding, DeclContext *Ctx, > bool InBaseClass) override { > bool Accessible = true; > - if (Ctx) > - Accessible = Results.getSema().IsSimplyAccessible(ND, Ctx); > + if (Ctx) { > + DeclContext *AccessingCtx = Ctx; > + // If ND comes from a base class, set the naming class back to the > + // derived class if the search starts from the derived class (i.e. > + // InBaseClass is true). > + // > + // Example: > + // class B { protected: int X; } > + // class D : public B { void f(); } > + // void D::f() { this->^; } > + // The completion after "this->" will have `InBaseClass` set to > true and > + // `Ctx` set to "B", when looking up in `B`. We need to set the > actual > + // accessing context (i.e. naming class) to "D" so that access > can be > + // calculated correctly. > + if (InBaseClass && isa<CXXRecordDecl>(Ctx)) { > + CXXRecordDecl *RC = nullptr; > + // Get the enclosing record. > + for (DeclContext *DC = CurContext; !DC->isFileContext(); > + DC = DC->getParent()) { > + if ((RC = dyn_cast<CXXRecordDecl>(DC))) > + break; > + } > + if (RC) > + AccessingCtx = RC; > + } > + Accessible = Results.getSema().IsSimplyAccessible(ND, > AccessingCtx); > + } > > ResultBuilder::Result Result(ND, Results.getBasePriority(ND), > nullptr, > false, Accessible, FixIts); > > Modified: cfe/trunk/test/Index/complete-access-checks.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/complete-access-checks.cpp?rev=337453&r1=337452&r2=337453&view=diff > > ============================================================================== > --- cfe/trunk/test/Index/complete-access-checks.cpp (original) > +++ cfe/trunk/test/Index/complete-access-checks.cpp Thu Jul 19 06:32:00 > 2018 > @@ -36,10 +36,10 @@ void Y::doSomething() { > > // CHECK-SUPER-ACCESS: CXXMethod:{ResultType void}{TypedText > doSomething}{LeftParen (}{RightParen )} (34) > // CHECK-SUPER-ACCESS: CXXMethod:{ResultType void}{Informative > X::}{TypedText func1}{LeftParen (}{RightParen )} (36) > -// CHECK-SUPER-ACCESS: CXXMethod:{ResultType void}{Informative > X::}{TypedText func2}{LeftParen (}{RightParen )} (36) (inaccessible) > +// CHECK-SUPER-ACCESS: CXXMethod:{ResultType void}{Informative > X::}{TypedText func2}{LeftParen (}{RightParen )} (36) > // CHECK-SUPER-ACCESS: CXXMethod:{ResultType void}{Informative > X::}{TypedText func3}{LeftParen (}{RightParen )} (36) (inaccessible) > // CHECK-SUPER-ACCESS: FieldDecl:{ResultType int}{Informative > X::}{TypedText member1} (37) > -// CHECK-SUPER-ACCESS: FieldDecl:{ResultType int}{Informative > X::}{TypedText member2} (37) (inaccessible) > +// CHECK-SUPER-ACCESS: FieldDecl:{ResultType int}{Informative > X::}{TypedText member2} (37) > // CHECK-SUPER-ACCESS: FieldDecl:{ResultType int}{Informative > X::}{TypedText member3} (37) (inaccessible) > // CHECK-SUPER-ACCESS: CXXMethod:{ResultType Y &}{TypedText > operator=}{LeftParen (}{Placeholder const Y &}{RightParen )} (79) > // CHECK-SUPER-ACCESS: CXXMethod:{ResultType X &}{Text X::}{TypedText > operator=}{LeftParen (}{Placeholder const X &}{RightParen )} (81) > @@ -87,3 +87,26 @@ void f(P x, Q y) { > // CHECK-USING-ACCESSIBLE: ClassDecl:{TypedText Q}{Text ::} (75) > // CHECK-USING-ACCESSIBLE: CXXDestructor:{ResultType void}{Informative > P::}{TypedText ~P}{LeftParen (}{RightParen )} (81) > // CHECK-USING-ACCESSIBLE: CXXDestructor:{ResultType void}{TypedText > ~Q}{LeftParen (}{RightParen )} (79) > + > +class B { > +protected: > + int member; > +}; > + > +class C : private B {}; > + > + > +class D : public C { > + public: > + void f(::B *b); > +}; > + > +void D::f(::B *that) { > + // RUN: c-index-test -code-completion-at=%s:106:9 %s | FileCheck > -check-prefix=CHECK-PRIVATE-SUPER-THIS %s > + this->; > +// CHECK-PRIVATE-SUPER-THIS: FieldDecl:{ResultType int}{Informative > B::}{TypedText member} (37) (inaccessible) > + > + // RUN: c-index-test -code-completion-at=%s:110:9 %s | FileCheck > -check-prefix=CHECK-PRIVATE-SUPER-THAT %s > + that->; > +// CHECK-PRIVATE-SUPER-THAT: FieldDecl:{ResultType int}{TypedText member} > (35) (inaccessible) > +} > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > -- Regards, Ilya Biryukov
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits