ilya-biryukov added inline comments.

================
Comment at: clangd/XRefs.cpp:201
   std::vector<MacroDecl> MacroInfos = DeclMacrosFinder->takeMacroInfos();
+  if (!MacroInfos.empty()) {
+    for (auto Item : MacroInfos) {
----------------
hokein wrote:
> ilya-biryukov wrote:
> > I wonder whether we should fix the `DeclrationAndMacrosFinder` to not 
> > return declarations coming from macro instantiations instead.
> > There are other clients (e.g. document highlights) that will probably break 
> > in the same way.
> > 
> > Do you think it would be possible and it would make sense for 
> > `DeclrationAndMacrosFinder` to only return a macro in this case and not 
> > return Decls coming from macro expansions?
> I thought about it initially, but the information provided 
> `handleDeclOccurrence` is limited...the occurrence location (`FID` and 
> `Offset`) is expansion location (which is not always we want). That being 
> said, when GoToDefinition on a macro, all declarations inside the macro body 
> will be picked up.
> 
> In document hover implementation, we also use the same mechanism to avoid 
> this problem :(
> 
As discussed offline, we should probably move this logic to 
`DeclrationAndMacrosFinder` so that all its clients (hover, documentHighlights, 
findDefinitions) are consistent.
Having a single function in `DeclrationAndMacrosFinder` that returns results 
(currently there are two: `takeDecls()` and `takeMacros()`) would allow that 
with minimal changes.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44293



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

Reply via email to