[PATCH] D122698: [clangd] Add support to extract method for ExtractFunction Tweak

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 4 07:03:54 PDT 2022

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks, this looks great! Just a couple of nits left.
Appreciate you fixing the out-of-line `ns::f()` case too. It's easier to understand the fixed logic than the broken logic.

I guess you don't have commit access yet, I can land this for you if you like. Let me know your preferred name/email for attribution.

Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:765
+  ExtractedFunc.ForwardDeclarationSyntacticDC = ExtractedFunc.SemanticDC =
+      ExtractedFunc.SyntacticDC = ExtZone.EnclosingFunction->getDeclContext();
You're setting/resetting these in lots of different places, but no need for that:

SyntacticDC = EnclosingFunction->getLexicalDeclContext()
SemanticDC = getDeclContext();

and set ForwardDeclarationSyntacticDC in captureMethodInfo(), leave it null if this isn't a method.

Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:770
+  if (isa<CXXMethodDecl>(ExtZone.EnclosingFunction)) {
+    const auto *Method =
this does one check, and is idiomatic in LLVM:

if (const auto *Method = llvm::dyn_cast<...>(...))



More information about the cfe-commits mailing list