hokein added inline comments.
================
Comment at: clangd/XRefs.cpp:201
std::vector<MacroDecl> MacroInfos = DeclMacrosFinder->takeMacroInfos();
+ if (!MacroInfos.empty()) {
+ for (auto Item : MacroInfos) {
----------------
ilya-biryukov wrote:
> 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.
As discussed, made the workaround in `Finder` to keep the change minimal, no
changes are required in client side.
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
