[MDEV-20103] add a class similar to std::span Created: 2019-07-19 Updated: 2019-08-05 Resolved: 2019-08-05 |
|
| Status: | Closed |
| Project: | MariaDB Server |
| Component/s: | Storage Engine - InnoDB |
| Fix Version/s: | 10.2.27, 10.3.18, 10.4.8 |
| Type: | Task | Priority: | Major |
| Reporter: | Eugene Kosov (Inactive) | Assignee: | Eugene Kosov (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Description |
|
https://en.cppreference.com/w/cpp/container/span This should be used instead of a such arguments T *ptr, size_t size, const std::vector<T> &v. Especially pointer and size pair is harmful: easy to use incorrect, hard to read the code. Class is non-owning! It's just a reference to something. Do not use it for strings! Another things exists for strings: https://en.cppreference.com/w/cpp/string/basic_string_view Suggestion to use are here https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines |
| Comments |
| Comment by Marko Mäkelä [ 2019-07-23 ] |
|
Looks good. I understood that the "not for strings" is referring to usage like span<char>(name, strlen(name)), but the example is an array of pointers to NUL-terminated strings. |
| Comment by Eugene Kosov (Inactive) [ 2019-07-25 ] |
|
> I wonder why we could not use span<const char*>(column_names, i). Why would it be any different from an array of pointers to something else than char? https://github.com/MariaDB/server/commit/dc9d18587fec459a32cdb5fd63454069d864d94f#r34441565 > I understood that the "not for strings" is referring to usage like span<char>(name, strlen(name)) Yes, exactly. |
| Comment by Marko Mäkelä [ 2019-07-25 ] |
|
Thanks, this is OK to push to 10.2, maybe after 10.2.27 has been released. |