ilya-biryukov added a comment.
Thanks for looking into this. Would be cool to get this supported after the
proposal is finalized.
================
Comment at: clangd/Protocol.h:891
+
+ std::vector<TypeHierarchyResult> Parents;
+
----------------
What is the proposal to add children (i.e. overriden methods) to the hierarchy?
This seems like a place where we might want to have multiple requests from the
clients when expanding the nodes.
================
Comment at: clangd/XRefs.cpp:669
+ const CXXMethodDecl *Candidate) {
+ // FIXME: How do I determine if Method overrides Candidate?
+
----------------
simark wrote:
> kadircet wrote:
> > https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp
> >
> > you can check this sample, which simply traverses all base classes and gets
> > methods with the same name, then checks whether one is overload of the
> > other. If it they are not overloads then one in the base classes gets
> > overriden by the other.
> >
> >
> > Another approach could be to search for one method in others
> > overriden_methods.
> >
> > But they are both inefficient, I would suggest calling these functions only
> > when one of them is defined in the base class of other then you can just
> > check for name equality and not being an overload.
> > https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp
>
> Did you mean to link to a particular line?
>
> > you can check this sample, which simply traverses all base classes and gets
> > methods with the same name, then checks whether one is overload of the
> > other. If it they are not overloads then one in the base classes gets
> > overriden by the other.
>
> > Another approach could be to search for one method in others
> > overriden_methods.
>
> I have tried using `overriden_methods`, but it only contains methods marked
> virtual. For this feature, I would like to consider non-virtual methods too.
>
> > But they are both inefficient, I would suggest calling these functions only
> > when one of them is defined in the base class of other then you can just
> > check for name equality and not being an overload.
>
> I am not sure I understand, but maybe it will be clearer when I will see the
> sample.
See the code that actually computes a list for `override_methods()`:
Sema::AddOverriddenMethods.
================
Comment at: clangd/XRefs.cpp:732
+ // If this is a variable, use the type of the variable.
+ CXXRD = VD->getType().getTypePtr()->getAsCXXRecordDecl();
+ } else if (Method) {
----------------
Why special-case the variables? Maybe just return empty results for consistency
with other use-cases (typedefs, etc)?
================
Comment at: clangd/XRefs.cpp:741
+ if (CXXRD)
+ return getTypeHierarchy(CXXRD, Method, SourceMgr);
+
----------------
Maybe also return all base types for classes?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51311
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
