nridge added a comment. Thanks for the patch and the thorough testcase!
I wonder if we should make this a bit more strict: if the macro expansion itself is an expression, show the value of //that// expression, but not e.g. an enclosing expression. Otherwise, the `Definition` field of the hover (which shows the tokens of the macro expansion) and the `Value` and `Type` fields (which show the value and type of a larger expression) could be out of sync and confusing. A (somewhat contrived) example would be: #define PLUS_TWO + 2 int x = 40 PLUS_TW^O; // huh, the value of "+ 2" is "42"? Of your existing testcases, I think the only one this stricter rule would break is #6 (`define_lambda_begin`), and I think that's fine. ================ Comment at: clang-tools-extra/clangd/Hover.cpp:728 } + SelectionTree::createEach( + AST.getASTContext(), AST.getTokens(), SM.getFileOffset(Tok.location()), ---------------- Why `createEach()` when the [non-macro case](https://searchfox.org/llvm/rev/be9c91843bab5bb46574c27836bfcd9ad6fc9ef5/clang-tools-extra/clangd/Hover.cpp#1270) uses `createRight()`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148457/new/ https://reviews.llvm.org/D148457 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
