hokein added inline comments.
================
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:
> hokein wrote:
> > 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.
> i don't think the orderings in the source vs test is that relevant, but SGTM
> if it helps you. mainly because:
> - as it might be harder in future to trigger diagnostics in the order we want
> - it is an invariant hard to maintain without at least the reviewer or author
> of changes knowing about(and remembering) it
yeah, it does help me, especially when I start adding more diagnostics.
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
