Skip to content

Conversation

@ckandeler
Copy link
Member

@ckandeler ckandeler commented Nov 19, 2024

...in DefineOutline tweak.
E.g. moving the following definition:
template<typename T> struct S { T f^oo() const { return T(); } };
would result in:
template<typename T> S<T>::T S::foo() const { return T(); }
instead of:
template<typename T> T S::foo() const { return T(); }

...in DefineOutline tweak.
E.g. moving the following definition:
  template<typename T> struct S { T f^oo() const { return T(); } };
would result in:
  // ...
  template<typename T> S<T>::T S::foo() const { return T(); }
instead of:
  template<typename T> T S::foo() const { return T(); }
@llvmbot
Copy link
Member

llvmbot commented Nov 19, 2024

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clangd

Author: Christian Kandeler (ckandeler)

Changes

...in DefineOutline tweak.
E.g. moving the following definition:
template<typename T> struct S { T f^oo() const { return T(); } };
would result in:
// ...
template<typename T> S<T>::T S::foo() const { return T(); }
instead of:
template<typename T> T S::foo() const { return T(); }


Full diff: https://github.com/llvm/llvm-project/pull/116821.diff

2 Files Affected:

  • (modified) clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp (+2)
  • (modified) clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp (+6-6)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index 2a96b305bd7a25..789c10bdd4822a 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -239,6 +239,8 @@ getFunctionSourceCode(const FunctionDecl *FD, const DeclContext *TargetContext,
           return;
 
         for (const NamedDecl *ND : Ref.Targets) {
+          if (ND->getKind() == Decl::TemplateTypeParm)
+            return;
           if (ND->getDeclContext() != Ref.Targets.front()->getDeclContext()) {
             elog("Targets from multiple contexts: {0}, {1}",
                  printQualifiedName(*Ref.Targets.front()),
diff --git a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
index 1af1bc31bf6486..d2d2ae9e7bb610 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp
@@ -411,14 +411,14 @@ inline typename O1<T, U...>::template O2<V, A>::E O1<T, U...>::template O2<V, A>
           R"cpp(
             struct Foo {
               template <typename T, bool B = true>
-              void ^bar() {}
+              T ^bar() { return {}; }
             };)cpp",
           R"cpp(
             struct Foo {
               template <typename T, bool B = true>
-              void bar() ;
+              T bar() ;
             };template <typename T, bool B>
-inline void Foo::bar() {}
+inline T Foo::bar() { return {}; }
 )cpp",
           ""},
 
@@ -426,14 +426,14 @@ inline void Foo::bar() {}
       {
           R"cpp(
             template <typename T> struct Foo {
-              template <typename U> void ^bar(const T& t, const U& u) {}
+              template <typename U> T ^bar(const T& t, const U& u) { return {}; }
             };)cpp",
           R"cpp(
             template <typename T> struct Foo {
-              template <typename U> void bar(const T& t, const U& u) ;
+              template <typename U> T bar(const T& t, const U& u) ;
             };template <typename T>
 template <typename U>
-inline void Foo<T>::bar(const T& t, const U& u) {}
+inline T Foo<T>::bar(const T& t, const U& u) { return {}; }
 )cpp",
           ""},
   };

Copy link
Collaborator

@HighCommander4 HighCommander4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ckandeler ckandeler merged commit 5310855 into llvm:main Nov 20, 2024
11 checks passed
@ckandeler ckandeler deleted the fix-qualified-template-param branch November 20, 2024 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants