r337453 - [CodeComplete] Fix accessibilty of protected members from base class.
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 30 08:22:05 PDT 2018
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 at 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
--
Regards,
Ilya Biryukov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180730/18823ec4/attachment-0001.html>
More information about the cfe-commits
mailing list