extract common code of growVector and copyAsPlain into helper

[ben-schwen]

Closes #6908

It dedoubles the code, but due to the recursive call of copyAsPlain, I'm not sure its really cleaner.

Maybe we should add a comment in both functions, linking to each other to not forget updating the code of one, if we update code of the other, and close the issue?

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.02%. Comparing base (0216983) to head (4afc587).
⚠️ Report is 9 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7602      +/-   ##
==========================================
+ Coverage   99.00%   99.02%   +0.02%     
==========================================
  Files          87       87              
  Lines       16893    16890       -3     
==========================================
+ Hits        16725    16726       +1     
+ Misses        168      164       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

• HEAD=unify_growVector_copyAsPlain stopped early for DT[by,verbose=TRUE] improved in #6296
  

Generated via commit 4afc587

Download link for the artifact containing the test results: ↓ atime-results.zip

Task	Duration
R setup and installing dependencies	3 minutes and 6 seconds
Installing different package versions	43 seconds
Running and plotting the test cases	4 minutes and 24 seconds

[aitap]

According to gcc -fstack-usage, copyVectorElements() now costs 48 bytes per function call, and copyAsPlain() does 32. On master, it's 48 bytes per copyAsPlain(). Maybe the recursive call to copyAsPlain is not so bad, since it shouldn't be on any hot paths, and R asks for a very large stack anyway.

As a minor improvement, it may be worth switching this bit of code from int64_t indices to R_xlen_t.

[MichaelChirico]

Suggested change

	copyVectorElements(newx, x, (R_xlen_t)len, false, __func__);
	copyVectorElements(newx, x, (R_xlen_t)len, /*deep_copy=*/false, __func__);

inspiration: https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html

[MichaelChirico]

I think the slight added complication for the "rarer" VECSXP case is worth it for the unified logic of the common atomic vector cases.