aaron.ballman 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.
----------------
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.)
================
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.
----------------
ioeric wrote:
> aaron.ballman wrote:
> > 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`?
> It's a bit messy, but this is roughly how lookup inside a class, say D,works:
> 1. lookup inside the class (InBaseClass = false, Context=D).
> 2. Run the same lookup on all base classes (InBaseClass=true, Context=B).
> (https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaLookup.cpp#L3584)
>
> So, `InBaseClass` would be true when a lookup starts from a derived class.
Ah, thank you for the explanation; that helps.
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
