[clang-tools-extra] [clangd] Extract to function: Do not reject unconditionally in C files (PR #228465)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

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

Author: Christian Kandeler (ckandeler)

<details>
<summary>Changes</summary>

C functions can be extracted just fine if the arguments are unmodified
scalars. All other cases would require parameters of pointer type and
thus changes to the extracted code.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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


2 Files Affected:

- (modified) clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp (+7-4) 
- (modified) clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp (+37-3) 


``````````diff
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
index 33a92daa07ccd..ccef8f88f90b6 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -993,7 +993,7 @@ CapturedZoneInfo captureZoneInfo(const ExtractionZone &ExtZone) {
 // FIXME: Check if the declaration has a local/anonymous type
 bool createParameters(NewFunction &ExtractedFunc,
                       const CapturedZoneInfo &CapturedInfo,
-                      const ASTContext &Context) {
+                      const ASTContext &Context, const LangOptions &LangOpts) {
   for (const auto &KeyVal : CapturedInfo.DeclInfoMap) {
     const auto &DeclInfo = KeyVal.second;
     // If a Decl was Declared in zone and referenced in post zone, it
@@ -1035,6 +1035,11 @@ bool createParameters(NewFunction &ExtractedFunc,
         TypeInfo.addConst();
       }
     }
+
+    // Cannot extract in a C file if we'd have to pass by reference.
+    if (IsPassedByReference && !LangOpts.CPlusPlus)
+      return false;
+
     // We use the index of declaration as the ordering priority for parameters.
     ExtractedFunc.Parameters.push_back({std::string(VD->getName()), TypeInfo,
                                         IsPassedByReference,
@@ -1134,7 +1139,7 @@ llvm::Expected<NewFunction> getExtractedFunction(ExtractionZone &ExtZone,
 
   ExtractedFunc.CallerReturnsValue = CapturedInfo.AlwaysReturns;
   if (!createParameters(ExtractedFunc, CapturedInfo,
-                        ExtZone.EnclosingFunction->getASTContext()) ||
+                        ExtZone.EnclosingFunction->getASTContext(), LangOpts) ||
       !generateReturnProperties(ExtractedFunc, *ExtZone.EnclosingFunction,
                                 CapturedInfo))
     return error("Too complex to extract.");
@@ -1209,8 +1214,6 @@ bool hasReturnStmt(const ExtractionZone &ExtZone) {
 
 bool ExtractFunction::prepare(const Selection &Inputs) {
   const LangOptions &LangOpts = Inputs.AST->getLangOpts();
-  if (!LangOpts.CPlusPlus)
-    return false;
   const Node *CommonAnc = Inputs.ASTSelection.commonAncestor();
   const SourceManager &SM = Inputs.AST->getSourceManager();
   auto MaybeExtZone = findExtractionZone(CommonAnc, SM, LangOpts);
diff --git a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
index 0f20fb218d07a..75828f143b9c4 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -56,9 +56,6 @@ TEST_F(ExtractFunctionTest, FunctionTest) {
   EXPECT_THAT(
       apply("#define RETURN_IF_ERROR(x) if (x) return\nRETU^RN_IF_ERROR(4);"),
       StartsWith("unavailable"));
-
-  FileName = "a.c";
-  EXPECT_THAT(apply(" for([[int i = 0;]];);"), HasSubstr("unavailable"));
 }
 
 TEST_F(ExtractFunctionTest, FileTest) {
@@ -1044,6 +1041,43 @@ TEST_F(ExtractFunctionTest, VolatileScalar) {
               HasSubstr("extracted(const volatile int &V)"));
 }
 
+TEST_F(ExtractFunctionTest, CFileAllowUnmodifiedScalar) {
+  FileName = "a.c";
+  Context = File;
+  EXPECT_THAT(apply(R"cpp(
+      int i;
+      void foo() {
+         int j = 0;
+         [[i = j;]]
+    })cpp"),
+              HasSubstr("extracted(int j)"));
+}
+
+TEST_F(ExtractFunctionTest, CFileRejectModifiedScalar) {
+  FileName = "a.c";
+  Context = File;
+  EXPECT_EQ(apply(R"cpp(
+      void foo() {
+         int j;
+         [[j = 0;]]
+    })cpp"),
+            "fail: Too complex to extract.");
+}
+
+TEST_F(ExtractFunctionTest, CFileRejectUnmodifiedStruct) {
+  FileName = "a.c";
+  Context = File;
+  EXPECT_EQ(apply(R"cpp(
+      struct pair { int v1; int v2; };
+      int i;
+      void foo() {
+         struct pair p;
+         p.v1 = 0;
+         [[i = p.v1;]]
+    })cpp"),
+            "fail: Too complex to extract.");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

``````````

</details>


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