sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land.
================ Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:648 + {"bool operator==(const Foo &) const noexcept = default", + Rel::TemplateInstantiation}); +} ---------------- this template pattern vs instantiation is really surprising, but it's a reasonable analogy, I don't see any particular problems. (Also, `operator<=>` looks like a template over whatever `=` is!) Do you know whether this means go-to-definition gives you two options to navigate to? (No need to write a test, just curious) ================ Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:4051 +TEST(Hover, RewrittenBinaryOperatorSpaceshipMassberg) { + Annotations T(R"cpp( ---------------- massberg wrote: > massberg wrote: > > sammccall wrote: > > > sammccall wrote: > > > > no need to sign your work :-) > > > can you add this to HoverTest__All instead? That way we test all details > > > of the hover card > > Upps, sorry. > > can you add this to HoverTest__All instead? That way we test all details of > > the hover card > > The (Hover, All) test tests with `std=c++17` while this test tests c++20 > features. > We could add an additional field with the version to the struct in the > (Hover, All) test. > Or add a (Hover, All_Cpp20) test for testing C++20 (what is probably not > worth at the moment with just one test requiring C++20). Does anything break if you switch everything to C++20? The intention of "std=c++17" there was certainly "not 14", rather than "not 20" :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153331/new/ https://reviews.llvm.org/D153331 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits