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

kadir çetinkaya via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 21 06:25:34 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()) {
----------------
kadircet wrote:

yes, `getEnclosingNamespaceContext` can return `TranslationUnitDecl`, e.g. for:
```
struct Foo { void b^ar() {} };
```

and things are getting messy in that scenario, I missed the part around getting source locations, it'll actually be invalid in that case.

so what about changing this logic to get to enclosing decl instead (i.e. TagDecl) and insert right after its end location?

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


More information about the cfe-commits mailing list