[PATCH] D66647: [clangd] DefineInline action apply logic with fully qualified names

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:339
+
+    const tooling::Replacement SemiColonToFuncBody(SM, *SemiColon, 1,
+                                                   *QualifiedBody);
----------------
s/SemiColon/Semicolon


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:323
+
+    auto SemiColon = getSemicolonForDecl(Target);
+    if (!SemiColon) {
----------------
ilya-biryukov wrote:
> NIT: s/SemiColon/Semicolon
This one is still there.


================
Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1332
+  template <>
+  void fo^o<char>(char p) {
+    return;
----------------
kadircet wrote:
> ilya-biryukov wrote:
> > How is this test different from the previous one?
> > Is it just trying to make sure we don't always move to the first function body?
> yes that's the point, adding a comment.
Thanks!


================
Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:1374
+  )cpp");
+}
+
----------------
kadircet wrote:
> ilya-biryukov wrote:
> > Do we test the following case anywhere?
> > ```
> > template <class T> void foo();
> > template <> void ^foo<int>() { // should not be available
> >   return;
> > }
> > ```
> Yes, there is once check in availability tests for:
> ```
>   EXPECT_UNAVAILABLE(R"cpp(
>     template <typename T> void foo();
> 
>     template<> void f^oo<int>() {
>     })cpp");
> ```
Ah, nice, I missed it. Thanks for pointing it out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66647