[PATCH] D49421: [CodeComplete] Fix accessibilty of protected members from base class.
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 18 05:07:34 PDT 2018
aaron.ballman added a reviewer: rsmith.
aaron.ballman added a comment.
Adding Richard to see if he agrees with the direction taken.
================
Comment at: lib/Sema/SemaAccess.cpp:1871-1873
+ // The access should be AS_none as we don't know how the member was
+ // accessed - `AccessedEntity::getAccess` describes what access was used to
+ // access an entity.
----------------
This seems to contradict the function-level comment that says the check seems to only take the access specifiers into account and not how the member is accessed.
================
Comment at: lib/Sema/SemaCodeComplete.cpp:1307
+ if (Ctx) {
+ auto *AccessingCtx = Ctx;
+ // If ND comes from a base class, set the naming class back to the
----------------
Please do not use `auto` as the type is not spelled out in the initialization.
================
Comment at: lib/Sema/SemaCodeComplete.cpp:1316-1317
+ // void D::f() { this->^; }
+ // The completion after "this->" will have `InBaseClass` set to true and
+ // `Ctx` set to "B". We need to set the actual accessing context (i.e.
+ // naming class) to "D" so that access can be calculated correctly.
----------------
This makes it sound like the caller has messed something up, but I'm also pretty unfamiliar with the code completion logic. Why would `InBaseClass` be true for that completion point and why would the context be set to `B`?
================
Comment at: lib/Sema/SemaCodeComplete.cpp:1319
+ // naming class) to "D" so that access can be calculated correctly.
+ if (InBaseClass && llvm::isa<CXXRecordDecl>(Ctx)) {
+ CXXRecordDecl *RC = nullptr;
----------------
You can drop the `llvm::`
================
Comment at: lib/Sema/SemaCodeComplete.cpp:1322
+ // Get the enclosing record.
+ for (auto *DC = CurContext; !DC->isFileContext();
+ DC = DC->getParent()) {
----------------
Please don't use `auto` here either.
================
Comment at: lib/Sema/SemaCodeComplete.cpp:1324
+ DC = DC->getParent()) {
+ if ((RC = llvm::dyn_cast<CXXRecordDecl>(DC)))
+ break;
----------------
You can drop the `llvm::`
Repository:
rC Clang
https://reviews.llvm.org/D49421
More information about the cfe-commits
mailing list