[PATCH] D70008: [clangd] Store xref for Macros in ParsedAST.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

looks like you forgot to upload the latest patch.



================
Comment at: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp:80
+        << Test;
+    EXPECT_THAT(collectKnownReferences(AST.getMacros()), AreMacroRefsFrom(T))
+        << Test;
----------------
usaxena95 wrote:
> 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.
>   
I see, having a complex `Matcher <std::vector<Matcher <std::vector<Range>>>>` would hurt code readability, and the error message is also not friendly when there are failed testcases.

How about extending it like below? To improve the error message, I think we should do a string comparison: the test annotation v.s the raw code with the actual macro references annotated (we'd need to write some code to compute this actual annotated code, you can see SemanticHighlightingTests.cpp for reference).  

```
#define $1[[a$1^bc]] 1
#undef $1[[abc]]

#define $2[[a$2^bc]] 2
#undef $2[[abc]]

#ifdef $Unknown[[abc]]
#endif
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70008/new/

https://reviews.llvm.org/D70008