-
Notifications
You must be signed in to change notification settings - Fork 15.2k
Description
The problem
The rule cppcoreguidelines-pro-bounds-pointer-arithmetic triggers when indexing argv in the main function, as it considers this to be unsafe pointer arithmetic. However, in this case, argv is a special array with a predictable size (argc) that could benefit from a specialized fix to make it's access safe.
The Solution
Since std::span is a safer way to handle array bounds and provides better index checking, it can be used as a more robust alternative to raw pointer arithmetic. Additionally it provides iterators, which can be used in all standard library functions that accept them. We can not use std::array here because the number of program arguments is not known at compile time.
Alternatively, a std::vector<std::string_view> could be used. Benefits would be the use of std::string_view, which is more flexible and type safe, and generally a lot easier to work with than raw c-strings. The downside is the dynamic memory allocation for the vector, rather than the minimal size overhead of the std::span.
In cases where a predictable function signature is not standardized like in main (ie. we don't know which parameter, if any (global, #define, weird parameter names and/or positions, etc), denotes the size), a helpful hint could be given instead, like "Wrap array into std::span and replace indexing with std::span::at()", or "Replace raw-array parameter with std::span for dynamically sized arrays, or std::array for statically sized arrays" or something of the sort.
Example
For main(int, char**) specifically, I propose that this, or a similar, quickfix be provided:
int main(int argc, char** argv)
{
const auto PROG_NAME = argv[0];
// ^~~~
// Do not use pointer arithmetic (clang-tidy cppcoreguidelines-pro-bounds-pointer-arithmetic)
}In this specific case, it should propose a quickfix, or at least a helpful hint, and replace the access with an std::span<char*>
// inserted headers
#include <cstddef> // for std::size_t
#include <span> // for std::span
int main(int argc, char** argv)
{
// inserted
const std::span<char*> ARGS(argv, static_cast<std::size_t>(argc));
// quickfix replaces all instances of argv[i] with ARGS.at(i)
const auto PROG_NAME = ARGS.at(0);
}or
// inserted headers
#include <cstddef> // for std::size_t
#include <string_view> // for std::string_view
#include <vector> // for std::vector
int main(int argc, char** argv)
{
// inserted
const std::vector<const std::string_view> ARGS(argv, static_cast<std::size_t>(argc));
// replace all instances of argv[i] with ARGS.at(i)
const auto PROG_NAME = ARGS.at(0);
}Example 2
void foo(int[] arr, const SomeObj& obj)
// ^~~~~
// Hint: replace int[] type with `std::span<int>` or `std::array<int, N>` here.
{
arr[obj.getArrIdx()] = 42; // array bounds can not be determined for an automated fix
// ^~~~~~~~~~~~~~~~~~~~
// Warning: cppcoreguidelines-pro-bounds-pointer-arithmetic
// consider replacing type of arr with std::array for a statically sized array, or std::span for a dynamically sized array
// Related: Hint: replace arr type here.
}Alternatives
Exclude argv indexing from cppcoreguidelines-pro-bounds-pointer-arithmetic, perhaps with an option in cppcoreguidelines-pro-bounds-pointer-arithmetic, and create a new rule for the specific case of indexing argv, to keep the rules simple and separate concerns.
Specific rule settings could then be given, for example to handle some of the considerations noted below (use at() or []; should type be const char* or char*; etc).
Notes
- If
std::argumentsproposal P3474 gets accepted, this needs to be revisited once implemented, as that would become the new, best way to address program arguments (std::arguments.at(1).string()). In that case this should probably be made into a new rule and excluded fromcppcoreguidelines-pro-bounds-pointer-arithmetic. - Consideration should be given if the inserted
ARGSspan should have the typeconst std::span<const char*>instead, or only ifargvis alreadyconstin themainsignature. std::span::at()will throw, consideration should be given if indexing should be done withoperator[]instead.- Considerations could be given if you want to possibly create an
const std::vector<const std::string_view>instead, though this has a tiny allocation and runtime overhead associated.