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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
