hokein added inline comments.
================ Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:709 ns::X *x; + auto& $type[[[]]a] = *x; x$access[[->]]f(); ---------------- kadircet wrote: > should this be `$type[[a]]` ? yeah, but this is the actual diagnostic range, though it is wired. ================ Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:715 auto TU = TestTU::withCode(Test.code()); + TU.ExtraArgs.push_back("-std=c++17"); auto Index = buildIndexWithSymbol( ---------------- kadircet wrote: > why do we need c++17 ? The newly-added test case is using C++17 feature (structure binding), needs this flag to suppress a diagnostic warning. ================ Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:738 + "Add include \"x.h\" for symbol ns::X"))), + AllOf(Diag(Test.range("nested"), + "incomplete type 'ns::X' named in nested name specifier"), ---------------- kadircet wrote: > can you move this back to original position ? yeah, I reordered the list. there are two orders: the one listed here, and the one listed in `IncludeFixer.cpp`. The motivation is to make them aligned, so that it would be easier to compare and spot the difference. The current state is not friendly to readers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88964/new/ https://reviews.llvm.org/D88964 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits