hokein added inline comments.
================
Comment at: clang-tools-extra/clangd/Macro.h:67
+ bool InMainFile = true;
+ std::vector<MainFileMacro> &MainFileMacros;
+};
----------------
ilya-biryukov wrote:
> hokein wrote:
> > ilya-biryukov wrote:
> > > Could we model in a way that avoids duplicating macro names on each
> > > occurrence?
> > > We had `StringSet Names` and `vector<SourceLocation> Locations`, let's
> > > keep it in the same way.
> > >
> > > We could group this into a struct to reduce boilerplate of transferring
> > > it around, obviously
> > > ```
> > > struct MainFileMacros {
> > > StringSet Names;
> > > vector<SourceLocation> Locations;
> > > };
> > > ```
> > yes, we don't need the name and location reletionship in this patch, but
> > we'd need this when implementing xrefs for macros. do you think we should
> > keep the same way, and do the change when we start implementing xrefs for
> > macros?
> Cross-references have to be more fine-grained and having only a name is not
> enough. Let's address this separately when/if we actually need this in xrefs.
> In particular, we want to handle macro re-definitions:
> ```
> #define FOO 123 // #1
> int a = FOO; // a reference to #1
>
> #define FOO 234 // #2
> int b = FOO; // a reference to #2
> ```
> Current model will merge these two together as both names are the same.
good point, agreed. I think we probably use the symbol id for macros, but it is
out-scope of this patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67496/new/
https://reviews.llvm.org/D67496
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
