[clang-tools-extra] [clangd] Extract to function: Pass unmodified scalar parameters by value (PR #227675)

[Aleksandr Platonov via cfe-commits]

================
@@ -1014,15 +1014,25 @@ bool createParameters(NewFunction &ExtractedFunc,
       return false;
     // Parameter qualifiers are same as the Decl's qualifiers.
     QualType TypeInfo = VD->getType().getNonReferenceType();
-    // Add const if it's not (conservatively) mutated in the zone: it's
-    // still passed by reference to avoid a copy, but the reference doesn't
-    // need to be mutable. Array types are never made const: mutating array
-    // elements through a non-const-ref loop variable or a decayed pointer
-    // argument is common and easy to miss conservatively, so we don't try.
-    if (!DeclInfo.IsPossiblyMutated && !TypeInfo->isArrayType())
-      TypeInfo.addConst();
     // FIXME: check if parameter will be a non l-value reference.
     bool IsPassedByReference = true;
+    if (!DeclInfo.IsPossiblyMutated) {
+      // A scalar (arithmetic, pointer, enumeration, ...) is at least as
+      // cheap to copy as to pass by reference, and less noisy. Any
+      // pre-existing const is dropped: it's a no-op on a by-value
+      // parameter, not a signal worth keeping.
+      if (TypeInfo->isScalarType()) {
+        IsPassedByReference = false;
+        TypeInfo.removeLocalConst();
----------------
ArcsinX wrote:

For this case:
```cpp
   void foo() {
      int A = 0;
      int &B = A;
      [[
        A = 1;
        bar(B);
      ]]
    }
```
`TypeInfo` for `B` is `int`, so is scalar type and we pass it by value.

After extraction behavior changes:
```cpp
    void extracted(int &A, int B) {
      A = 1;
      bar(B); // B is 0 here, but expected to be 1
    }
    
    void foo() {
      int A = 0;
      int &B = A;
      extracted(A, B);
    }
```

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