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

Reply via email to