[PATCH] D51311: (WIP) Type hierarchy

[Simon Marchi via Phabricator via cfe-commits]

simark added inline comments.


================
Comment at: clangd/Protocol.h:889
+  // Does this node implement the method targeted by the request?
+  bool DeclaresMethod;
+
----------------
kadircet wrote:
> I think comment and the name is in contradiction here, do you mean DefinesMethod?
Actually I think the comment is wrong.  Even if we only see a declaration of the method, it's enough to say that this type has its own version of the method.


================
Comment at: clangd/XRefs.cpp:669
+                          const CXXMethodDecl *Candidate) {
+  // FIXME: How do I determine if Method overrides Candidate?
+
----------------
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.


================
Comment at: clangd/XRefs.cpp:693
+
+    std::cerr << " >>> A parent is " << ParentDecl->getName().str()
+              << std::endl;
----------------
kadircet wrote:
> If you plan to keep it for later debugging info, consider using {d,v}log
Yes of course :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51311