usaxena95 marked 7 inline comments as done.
usaxena95 added inline comments.
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:898
+ // Handle macros.
+ if (auto Macro = locateMacroAt(Loc, AST.getPreprocessor())) {
+ if (auto MacroSID = getSymbolID(Macro->Name, Macro->Info, SM)) {
----------------
hokein wrote:
> usaxena95 wrote:
> > hokein wrote:
> > > this is not completed, it only shows the macro refs in main file, we
> > > still need to query the index to get the rest (but our index doesn't
> > > collect macro at the moment). I'd prefer not doing this in this patch
> > > until our index is ready.
> > >
> > > For testing, I think we can create a new test file for
> > > `CollectMainFileMacros`.
> > Yup. This patch is just for the main file. The index and the lookup will be
> > modified in the next patch. Wanted to keep this small.
> >
> this would enable the xref for macros, but the feature is not ready yet. I
> think we'd better add this code when everything is ready.
Removed looking up xref.
================
Comment at: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp:80
+ << Test;
+ EXPECT_THAT(collectKnownReferences(AST.getMacros()), AreMacroRefsFrom(T))
+ << Test;
----------------
hokein wrote:
> I might be missing something, I didn't get the motivation of using numbers in
> the annotation, the code seems just collecting all annotated ranges
> regardless the value of the number and compare to the actual ranges. it
> doesn't verify that e.g. in your 2rd cases, the two macros `abc` are
> different symbols.
>
> How about just verifying a single defined macro for each case? e.g. the below
> case, we try to get the symbol id (call `locatedMacroAt`) from the T.point(),
> retrieve the responding ranges from the MainFileMacros, compare to the
> annotated ranges.
>
> ```
> #define [[F^oo]] 1
> int abc = [[Foo]];
> #undef [[Foo]]
> ```
Since the output of CollectMacros is a mapping from SymbolID -> vector, I
wanted to verify that we form correct groups of ranges (grouped by SymbolID).
So a single macro per test case wouldn't be able to test this.
> I didn't get the motivation of using numbers in the annotation, the code
> seems just collecting all annotated ranges regardless the value of the number
> and compare to the actual ranges. it doesn't verify that e.g. in your 2rd
> cases, the two macros abc are different symbols.
We actually group the ranges by their annotation and form `Matcher
<std::vector<Matcher <std::vector<Range>>>>`. So it checks for the correct
grouping too.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70008/new/
https://reviews.llvm.org/D70008
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
