[clang-tools-extra] [clangd] Allow "move function body out-of-line" in non-header files (PR #69704)

Christian Kandeler via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 21 05:09:28 PST 2023


================
@@ -499,17 +473,64 @@ class DefineOutline : public Tweak {
       HeaderUpdates = HeaderUpdates.merge(*DelInline);
     }
 
-    auto HeaderFE = Effect::fileEdit(SM, SM.getMainFileID(), HeaderUpdates);
-    if (!HeaderFE)
-      return HeaderFE.takeError();
-
-    Effect->ApplyEdits.try_emplace(HeaderFE->first,
-                                   std::move(HeaderFE->second));
+    if (SameFile) {
+      tooling::Replacements &R = Effect->ApplyEdits[*CCFile].Replacements;
+      R = R.merge(HeaderUpdates);
+    } else {
+      auto HeaderFE = Effect::fileEdit(SM, SM.getMainFileID(), HeaderUpdates);
+      if (!HeaderFE)
+        return HeaderFE.takeError();
+      Effect->ApplyEdits.try_emplace(HeaderFE->first,
+                                     std::move(HeaderFE->second));
+    }
     return std::move(*Effect);
   }
 
+  // Returns the most natural insertion point for \p QualifiedName in \p
+  // Contents. This currently cares about only the namespace proximity, but in
+  // feature it should also try to follow ordering of declarations. For example,
+  // if decls come in order `foo, bar, baz` then this function should return
+  // some point between foo and baz for inserting bar.
+  llvm::Expected<InsertionPoint> getInsertionPoint(llvm::StringRef Contents,
+                                                   const Selection &Sel) {
+    // If the definition goes to the same file and there is a namespace,
+    // we should (and, in the case of anonymous namespaces, need to)
+    // put the definition into the original namespace block.
+    // Within this constraint, the same considerations apply as in
+    // the FIXME below.
+    if (SameFile) {
+      if (auto *Namespace = Source->getEnclosingNamespaceContext()) {
----------------
ckandeler wrote:

You mean this also covers the global namespace? I wasn't sure about that. But don't we need special handling for that case, then? As I'd assume it won't have a proper location?

https://github.com/llvm/llvm-project/pull/69704


More information about the cfe-commits mailing list