[clang-tools-extra] [clang-tidy]suggest use `std::span` as replacement of c array in C++20 for modernize-avoid-c-arrays (PR #108555)

Julian Schmidt via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 13 10:04:46 PDT 2024


================
@@ -1,9 +1,9 @@
-// RUN: %check_clang_tidy %s modernize-avoid-c-arrays %t -- \
+// RUN: %check_clang_tidy -std=c++17 %s modernize-avoid-c-arrays %t -- \
 // RUN:  -config='{CheckOptions: { modernize-avoid-c-arrays.AllowStringArrays: true }}'
 
 const char name[] = "name";
 const char array[] = {'n', 'a', 'm', 'e', '\0'};
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not declare C-style arrays, use std::array<> instead [modernize-avoid-c-arrays]
 
 void takeCharArray(const char name[]);
-// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: do not declare C-style arrays, use std::array<> instead [modernize-avoid-c-arrays]
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: do not declare C-style arrays, use std::vector<> instead [modernize-avoid-c-arrays]
----------------
5chmidti wrote:

When we have `T[]`, the size is most likely not evident, even for the developer that has semantic knowledge of their code that may not be the case, so `std::array` will be unlikely to be helpful. I'd suggest that the diagnostic suggest to users to use `std::vector` or `std::span` (as in provide both options in the message) in case the array is variable, and `std::array` otherwise. Maybe even mentioning `std::array` in the variable array case.

In the case of VLAs I agree on proposing to use `std::vector`, because it is known that the length is variable, even if the original array may have lived on the stack this makes sense to do IMO.

In the case of incomplete array types, we can't know if the original array/buffer was statically sized and or dynamically allocated, and therefore makes suggesting `std::vector` problematic, at least in some cases. Using `std::span` would be the appropriate facility in this case, but that only applies to >=C++20. Because `std::vector` may not be the desired solution to the issue, the check could have an option that enables the proposal of using `std::vector`, and also supply an option to suggest custom pre-C++20 wrappers (e.g., `gsl::span`). The suggestion of `std::vector` could be done as well, though `std::array` should be suggested as well in that case due to the uncertainty of stack vs heap allocation as well as statically sized vs variable sized.

Mabye the wording of the diagnostic could also be less in the direction of: this is the solution, and instead be reworded towards a suggestion: `you could use _ [or _] instead`.

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


More information about the cfe-commits mailing list