kadircet added inline comments.

================
Comment at: clangd/ProtocolHandlers.cpp:70
   Register("textDocument/hover", &ProtocolCallbacks::onHover);
+  Register("textDocument/typeHierarchy", &ProtocolCallbacks::onTypeHierarchy);
   Register("textDocument/documentSymbol", 
&ProtocolCallbacks::onDocumentSymbol);
----------------
LSP doesn't have such an entry, maybe we should use [[ 
https://microsoft.github.io/language-server-protocol/specification#workspace_executeCommand
 | workspace/executeCommand ]]. WDYT @ilya-biryukov 


================
Comment at: clangd/XRefs.cpp:669
+                          const CXXMethodDecl *Candidate) {
+  // FIXME: How do I determine if Method overrides Candidate?
+
----------------
ilya-biryukov wrote:
> 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.
> Did you mean to link to a particular line?


Yeah sorry, I was trying to link the function ilya mentioned.
https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp#L7615

Since you also mentioned checking non-virtual method as well you might wanna 
look at 
https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp#L7555 as 
well.

Also I got the chance to look the rest of the patch, as I mentioned above you 
are already checking two methods iff one lies in the others base classes. So 
you can simply check for name equality and not being an overload case.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51311



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to