[PATCH] D85028: [clangd] Support new/deleta operator in TargetFinder.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

> please note that this might require special handling for go-to-def. as go-to-def only jumps to canonical decl and in operator new's case the user provided one might not be canonical, whereas the canonical one is likely builtin without a source info.

this is a good point, I didn't pay much attention to this case. The global new/delete are a bit tricky, they are implicitly declared in every TU.

> LGTM if you are not planning to make this work with go-to-def now (or ever), please LMK.

My motivation of this patch is to fix class-specific new/delete operators (go-to-def already works). I added a FIXME in go-to-def. I guess that case happens rarely in practice.



================
Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:564
+  Code = R"cpp(
+    struct X {
+      static void *operator new(unsigned long s);
----------------
kadircet wrote:
> nit: you can simplify to (same for delete)
> 
> ```cpp
> void *operator new(unsigned long);
> void foo() { new int; }
> ```
I think global and class-specific new/delete operators are not the same thing, I would like to keep both of them. Added the global one.


================
Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:565
+    struct X {
+      static void *operator new(unsigned long s);
+    };
----------------
kadircet wrote:
> you might want to set --target to a fix triplet to ensure `size_t == unsigned long`
good catch, done. The test failed on the windows pre-merge test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85028/new/

https://reviews.llvm.org/D85028