[PATCH] D49421: [CodeComplete] Fix accessibilty of protected members from base class.

Eric Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 18 13:35:39 PDT 2018


ioeric added inline comments.


================
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.
----------------
aaron.ballman wrote:
> ioeric wrote:
> > aaron.ballman wrote:
> > > 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.
> > > not how the member is accessed.
> > It's not very clear to me what exactly this means.  But I think we are still not taking how member was accessed into account. The access in `DeclAccessPair` describes how a member was accessed, and this should be None instead of the access specifier of the member as we don't have this information here. 
> > 
> > I updated the wording in the comment to make this a bit clearer.
> I'm still wondering about the removal of the public access check -- if the declaration has a public access specifier, that early return saves work. So `Entity` may still require passing `AS_none` but we might want to continue to check `Decl->getAccess() == AS_public` for the early return?
> 
> (Drive-by nit that's not yours: `Decl` is the name of a type, so if you wanted to rename the parameter to something that isn't a type name, I would not be sad, but you're not required to do it.)
Added `Decl->getAccess() == AS_public` back to preserve the behavior.

s/Decl/Target/


Repository:
  rC Clang

https://reviews.llvm.org/D49421





More information about the cfe-commits mailing list