36

Lets show it in an example where we have a Data class with primary data, some kind of index that points to the primary data, and we also need to expose a const version the index.

class Data
{
public:
  const std::vector<int>& getPrimaryData() const { return this->primaryData; }
  const std::vector<int*>& getIndex() const { return this->index; }
private:
  std::vector<int> primaryData;
  std::vector<int*> index;
};

This is wrong, as the user can easily modify the data:

const Data& data = something.getData();
const std::vector<int*>& index = data.getIndex();
*index[0] = 5; // oups we are modifying data of const object, this is wrong

The reason of this is, that the proper type the Data::getIndex should be returning is:

const std::vector<const int*>&

But you can guess what happens when you try to write the method that way to "just convert the non-const variant to const variant":

// compiler error, can't convert std::vector<int*> to std::vector<const int*> these are unrelated types.
const std::vector<const int*>& getIndex() const { return this->index; }

As far as I know, C++ doesn't have any good solution to this problem. Obviously, I could just create new vector, copy the values from the index and return it, but that doesn't make any sense from the performance perspective.

Please, note that this is just simplified example of real problems in bigger programs. int could be a bigger object (Book lets say), and index could be index of books of some sort. And the Data might need to use the index to modify the book, but at the same time, provide the index to read the books in a const way.

John Kugelman
  • 349,597
  • 67
  • 533
  • 578
kovarex
  • 1,766
  • 18
  • 34
  • 2
    "oups we are modifing data of const object" you are not modifying data of a `const` object. `index[0]` is unchanged – 463035818_is_not_an_ai Apr 25 '22 at 10:04
  • Why would it be unchanged? – kovarex Apr 25 '22 at 10:06
  • why do you have a vector of pointers in the first place? You describe what you expect and how it does not work, but what exactly is "this problem" you are trying to solve? Can the member not be a `std::vector` ? Can you not return const iterators to the vector? – 463035818_is_not_an_ai Apr 25 '22 at 10:06
  • 2
    "Why would it be unchanged?" because `*some_ptr = 42` does not modify `some_ptr` – 463035818_is_not_an_ai Apr 25 '22 at 10:06
  • 1
    The member can't be std::vector because in the the inner logic of Data (or when non-const access is requiested), I use the index to actially modify the values. – kovarex Apr 25 '22 at 10:08
  • You have vector of pointers to mutable ints not vector of pointers to immutable ints. So do you expect C++ to change it to pointers to immutable ints? – Öö Tiib Apr 25 '22 at 10:08
  • Yes, I wan't exactly that, I want a way to convert it without having to make a copy. – kovarex Apr 25 '22 at 10:11
  • 1
    Will you ever be changing the indices via `index`? If not, store a `std::vector` internally and you can return it to callers without issue. If you are, it's questionable to return a vector of "const" pointers, implying the ints are immutable, when the values are subject to changing. As I see it, the difficulty you're having is exposing a design flaw in your API. – John Kugelman Apr 25 '22 at 10:12
  • 3
    [std::experimental::propagate_const](https://en.cppreference.com/w/cpp/experimental/propagate_const) might help: `std::vector> indexes;` – Jarod42 Apr 25 '22 at 10:16
  • Yes, I want to use the index to internally change the value, that is the core of the whole problem. And no, const doesn't mean immutable, so I don't see a problem with that. – kovarex Apr 25 '22 at 10:16
  • 13
    @kovarex The design itself is flawed so you won't find a good solution with plain c++. The dirty way is to use `const int*` internally and `const_cast` it away when write access is needed. A better solution would be to not pass a `const std::vector&` but instead give out the `const_iterators` on the data as read only access. – mkaes Apr 25 '22 at 10:23
  • Read https://isocpp.org/wiki/faq/const-correctness to understand how it works in C++ – Öö Tiib Apr 25 '22 at 10:25
  • 2
    I disagree with design flow in general, `const std::vector>` would be fine for op. It is just that pointer indirection don't propagate constness. – Jarod42 Apr 25 '22 at 10:26
  • 1
    I read all isocpp.org/wiki/faq/const-correctness and it doesn't help solving this problem in any way, and is all in line with this example showing how it is broken. – kovarex Apr 25 '22 at 10:31
  • 1
    Option 1: don't return a reference to an internal member variable, instead return a copy. Option 2: why does other code need to call this code? Put that functionality in this routine. The **Hollywood Principle** (also known as "Tell, Don't Ask" (TDA)). – Eljay Apr 25 '22 at 11:20
  • 1
    You can write an adapter class. – Sebastian Apr 26 '22 at 13:41

8 Answers8

49

In C++20, you can just return a std::span with elements of type const int*

#include <vector>
#include <span>

class Data
{
public:
  std::span<const int* const> getIndex() const { return this->index; }
private:
  std::vector<int*> index;
};

int main() {
  const Data data;
  const auto index = data.getIndex();
  *index[0] = 5;  // error: assignment of read-only location
}

Demo

康桓瑋
  • 33,481
  • 5
  • 40
  • 90
  • This is very specialised solution based on the simplified nature of the example using just std::vector. Once the index becomes anything more custom, this wouldn't work. – kovarex Apr 25 '22 at 11:08
  • 8
    This method works for *any* continuous range. If you don't use a continuous range to store the index, the other answers based on pointers won't work either (like the one you accepted). – 康桓瑋 Apr 25 '22 at 11:12
  • @康桓瑋: Accepted `propagate_const` might works for `std::list>`. – Jarod42 Apr 25 '22 at 11:43
19

Each language has its rules and usages... std::vector<T> and std::vector<const T> are different types in C++, with no possibility to const_cast one into the other, full stop. That does not mean that constness is broken, it just means it is not the way it works.

For the usage part, returning a full container is generally seen as a poor encapsulation practice, because it makes the implementation visible and ties it to the interface. It would be better to have a method taking an index and returning a pointer to const (or a reference to a pointer to const if you need it):

const int* getIndex(int i) const { return this->index[i]; }

This works, because a T* can be const_casted to a const T *.

Serge Ballesta
  • 143,923
  • 11
  • 122
  • 252
  • 2
    I beg to differ about the bad practice. I think the opposite, as bad practice is to unpack functionality of internal containers into the API, making class interfaces fat. In other words, imagine, that the index isn't just a std::vector, but a 3 layer structure of some complex indexing system. would you copy all the ways it can be accessed in to the surface layer of the Data class? For example, I couldn't use std::find_if and similar on the index anymore, and if I wanted to do it, I would have to implement iterators, begin/end methods in the Data class all for just allow proper ... – kovarex Apr 25 '22 at 10:23
  • 4
    @kovarex: You are right on one point: adding unnecessary complexity is always bad. In the real world, it is even the strength of experienced analysts and devs: find the correct balance between the complexity of the interface, the complexity of the implementation and the strict respect of best practices. Rules are not to be blindly obeyed, but ignoring them without a strong reason **is** bad... – Serge Ballesta Apr 25 '22 at 12:23
  • 6
    @kovarex ... Here you gave a simple example and I only answered about that example. IMHO, if you have a full example with all of its use cases and want to know how to improve it, you could try to post it to [Code Review](https://codereview.stackexchange.com/) to get an exhaustive review. – Serge Ballesta Apr 25 '22 at 12:27
9

The top answer, using ranges or spans, is a great solution, if you can use C++20 or later (or a library such as the GSL). If not, here are some other approaches.

Unsafe Cast

#include <vector>

class Data
{
public:
  const std::vector<const int>& getPrimaryData() const
  {
    return *reinterpret_cast<const std::vector<const int>*>(&primaryData);
  }

  const std::vector<const int* const>& getIndex()
  {
    return *reinterpret_cast<const std::vector<const int* const>*>(&index);
  }

private:
  std::vector<int> primaryData;
  std::vector<int*> index;
};

This is living dangerously. It is undefined behavior. At minimum, you cannot count on it being portable. Nothing prevents an implementation from creating different template overloads for const std::vector<int> and const std::vector<const int> that would break your program. For example, a library might add some extra private data member to a vector of non-const elements that it doesn’t for a vector of const elements (which are discouraged anyway).

While I haven’t tested this extensively, it appears to work in GCC, Clang, ICX, ICC and MSVC.

Smart Array Pointers

The array specialization of the smart pointers allows casting from std::shared_ptr<T[]> to std::shared_ptr<const T[]> or std::weak_ptr<const T[]>. You might be able to use std::shared_ptr as an alternative to std::vector and std::weak_ptr as an alternative to a view of the vector.

#include <memory>

class Data
{
public:
  std::weak_ptr<const int[]> getPrimaryData() const
  {
    return primaryData;
  }

  std::weak_ptr<const int* const[]> getIndex()
  {
    return index;
  }

private:
  std::shared_ptr<int[]> primaryData;
  std::shared_ptr<int*[]> index;
};

Unlike the first approach, this is type-safe. Unlike a range or span, this has been available since C++11. Note that you would not actually want to return an incomplete type with no array bound—that’s just begging for a buffer overflow vulnerability—unless your client knew the size of the array by some other means. It would primarily be useful for fixed-size arrays.

Subranges

A good alternative to std::span is a std::ranges::subrange, which you can specialize on the const_iterator member type of your data. This is defined in terms of a begin and end iterator, rather than an iterator and size, and could even be used (with modification) for a container with non-contiguous storage.

This works in GCC 11, and with clang 14 with -std=c++20 -stdlib=libc++, but not all other compilers (as of 2022):

#include <ranges>
#include <vector>

class Data
{
private:
   using DataType = std::vector<int>;
   DataType primaryData;
   using IndexType = std::vector<DataType::pointer>;
   IndexType index;

public:
  /* The types of views of primaryData and index, which cannot modify their contents.
   * This is a borrowed range. It MUST NOT OUTLIVE the Data, or it will become a dangling reference.
   */
  using DataView = std::ranges::subrange<DataType::const_iterator>;
  // This disallows modifying either the pointers in the index or the data they reference.
  using IndexView = std::ranges::subrange<const int* const *>;

  /* According to the C++20 standard, this is legal.  However, not all
   * implementations of the STL that I tested conform to the requirement that
   * std::vector::cbegin is contstexpr.
   */    
  constexpr DataView getPrimaryData() const noexcept
  {
    return DataView( primaryData.cbegin(), primaryData.cend() );
  }

  constexpr IndexView getIndex() const noexcept
  {
    return IndexView( index.data(), index.data() + index.size() );
  }
};

You could define DataView as any type implementing the range interface, such as a std::span or std::string_view, and client code should still work.

Toby Speight
  • 27,591
  • 48
  • 66
  • 103
Davislor
  • 14,674
  • 2
  • 34
  • 49
  • @康桓瑋 I’ve now fixed the type of `IndexView` so that `data,getIndex()[0] - 5;` fails with the error `returns a const value`, and `*data.getIndex()[0] = 5;` fails with `read-only variable is not assignable`.` – Davislor Apr 26 '22 at 06:37
  • *"This is living dangerously."* Are you the kind of person who considers accepting the Terms and Conditions without reading them to be living dangerously? Because it's not really any more dangerous. – user541686 Apr 27 '22 at 10:28
  • @user541686: Some people program not for fun, but in their jobs for important products or services. And in that environment the legal departments read contracts accurately. For good reason. – Sebastian Apr 27 '22 at 11:14
  • @user541686 There are things that are formally “undefined behavior” in C++ that I think are completely safe to use in practice, like `fflush(stdin)` or type-punning on a `union` of Plain Old Data. Every time a compiler has broken all the existing code that depends on something like those, it’s the language Standard that has changed to conform to what programs do in the real world. This time, I’m not as sure I have safety in numbers. – Davislor Apr 27 '22 at 14:28
  • 2
    @user541686 And, to be clear, the “danger” is not that you would get into some kind of trouble for not being a good little programmer. It’s that your code could break on a different version of the compiler or library. In theory, the language standards exist to prevent that from happening. – Davislor Apr 27 '22 at 17:53
  • Your first two solutions only mention `primaryData`, but `primaryData` already works correctly in the original code. It's `index` that's the problem. Your second solution (smart pointers) won't work at all in the `index` case, and that's no accident; it would make the type system unsound if it was allowed. (Also, the array specialization of `shared_ptr` has only existed since C++17) – benrg Apr 27 '22 at 19:19
  • @benrg All three solutions now implement both `getPrimaryData` and `getIndex`. Good suggestion! – Davislor Apr 27 '22 at 20:12
  • In the second suggestion, `std::weak_ptr` doesn't provide a way to know the array size, unlike the original `std::vector`. This makes the two methods unusable (without returning a size separately). – ugoren Apr 28 '22 at 06:39
  • @Sebastian: Type-punning a union of POD (which you're fine with) is *more* dangerous than this, not *less*. ([Here's an example](https://gcc.godbolt.org/z/fn5PPerYv) of it silently going wrong, courtesy of [here](https://stackoverflow.com/a/51228315/541686).) By contrast, the vector example you have, I think you would be *hard* pressed to find a realistic way to make it break on any compiler in any plausible way. I'm fairly confident that you'd have to go out of your way to write code that nobody would write, and which would trigger red flags for anybody reading it (unlike the union case). – user541686 Apr 28 '22 at 08:32
  • So while I do agree with you that `std::span` is probably a better idea, but I think "living dangerously" is quite an exaggeration of the risk. I wouldn't exactly recommend you send astronauts to space with code like this, but for daily apps... people write *far* more "dangerous" code on a regular basis without running into problems. – user541686 Apr 28 '22 at 08:35
  • 1
    @user541686 The bug in your code is that the arguments passed by reference to `foo` violate the language’s aliasing rules. You would get the same bug in C, where the language standard formally guarantees that type-punning through a `union` is safe (in the sense I meant, reading a different `union` member than was last written). It’s certainly possible to write code involving unions that has bugs. Today, I would write most C++ code where I used to type-pun with a `std::bit_cast`. – Davislor Apr 28 '22 at 10:17
  • @user541686 I agree that the `shared_ptr` implementation would only be useful for fixed-sized arrays. – Davislor Apr 28 '22 at 10:24
  • @user541686 So, for example, another kind of bug with “type-punning” would be to construct an object with a trap representation. That’s unsafe. Or you might have a `union` of objects of two different sizes, and that would also make type-punning unsafe. In any case, I probably agree that “living dangerously” means something more serious to you than this example. – Davislor Apr 28 '22 at 10:30
  • @user541686 Am I fine with type-puning by unions in C++? No. If I have said so anywhere (where did I?) I would have been wrong. Modern C++ has safe, comfortable, performant, easy-to-read alternatives like `bit_cast`. – Sebastian Apr 28 '22 at 10:49
  • @Sebastian: I think I quoted the wrong person in [this comment](/questions/71997744/how-can-i-propagate-const-when-returning-a-stdvectorint-from-a-const-method/72006642?noredirect=1#comment127275250_72006642), I didn't realize you weren't the one who wrote it. – user541686 Apr 28 '22 at 10:55
  • @Sebastian I would also use `std::bit_cast` today. Before it existed, I often preferred writing conversions as static single assignments involving a `union`, rather than a `reinterpret_cast` or `memcpy()`, for various reasons that are not relevant to this discussion. – Davislor Apr 28 '22 at 12:46
8

You could return a transforming view to the vector. Example:

auto getIndex() const {
    auto to_const = [](int* ptr) -> const int* {
        return ptr;
    };
    return this->index | std::views::transform(to_const);
}

Edit: std::span is simpler option.


If index contains pointers to elements of primaryData, then you could solve the problem by instead storing integers representing the indices of the currently pointed objects. Anyone with access to non-const primaryData can easily turn those indices to pointers to non-const, others cannot.

primaryData isn't stable,

If primaryData isn't stable, and index contains pointers to primaryData, then the current design is broken because those pointers would be invalidated. The integer index alternative fixes this as long as the indices remain stable (i.e. you only insert to back). If even the indices aren't stable, then you are using a wrong data structure. Linked list and a vector of iterators to the linked list could work.

eerorika
  • 232,697
  • 12
  • 197
  • 326
  • You are fixating too much on the technical part of the example, yes technically, the pointer to data would be invalidated, but this can be easily fixed if the data was just something different than a vector. That is not the core of the problem. – kovarex Apr 25 '22 at 11:09
7

You're asking for std::experimental::propagate_const. But since it is an experimental feature, there is no guarantee that any specific toolchain is shipped with an implementation. You may consider implementing your own. There is an MIT licensed implementation, however. After including the header:

using namespace xpr=std::experimental;
///...
std::vector<xpr::propagate_const<int*>> my_ptr_vec;

Note however that raw pointer is considered evil so you may need to use std::unique_ptr or std::shared_ptr. propagate_const is supposed to accept smart pointers as well as raw pointer types.

ComicSansMS
  • 51,484
  • 14
  • 155
  • 166
Red.Wave
  • 2,790
  • 11
  • 17
  • 11
    `Note however that raw pointer is considered evil` No it isn't. `you may need to use std::unique_ptr or std::shared_ptr` They shouldn't use those unless the pointers are owning. – eerorika Apr 25 '22 at 10:41
  • 1
    @eerorika there is no information about the context, where the op is using pointers. There's even a `std::experimental::observer_ptr` for use cases that you intend. So, that **evil** is a tad darker than I mentioned. – Red.Wave Apr 25 '22 at 10:49
  • Ok, this seems to be the technical solution to the problem, but honestly, it is kind of depressing, because it looks, like I would have to let the std::experimental::propage_const bloat to bleed outside the api to all of the code that uses that. So the incrimining line would look like this: const std::vector>& index = data.getIndex(); Honestly this amount of bloat seems to be probably even worst than not caring about const corectness – kovarex Apr 25 '22 at 11:05
  • 2
    @kovarex `auto& index = data.getIndex();` – 463035818_is_not_an_ai Apr 25 '22 at 11:06
  • I prefer to see my type names, so auto is not an option in situations like this. And also, you will end up in situation where you have to pass it around etc, so the explicit type will have to be written on a lot of places if you if you are fan of auto. – kovarex Apr 25 '22 at 11:10
  • 2
    @kovarex `using index_t = std::experimental:::propagate_const` and `const std::vector& index = data.getIndex()` – Artyer Apr 25 '22 at 11:17
  • 4
    @kovarex `typedef`, `using`, `decl_type`. `std::vector::reference`. A lot of ways to make life easier. I did alias the namespace to an abbreviation for very same reason. – Red.Wave Apr 25 '22 at 11:20
  • If the you use `typeinfo` to print the type, you can inherit a new type and use constructor forwarding. – Red.Wave Apr 25 '22 at 11:27
5

As mentioned in a comment, you can do this:

class Data
{
public:
  const std::vector<int>& getPrimaryData() const { return this->primaryData; }
  const std::vector<const int*>& getIndex() const { return this->index; }
private:
  std::vector<int> primaryData;
  std::vector<const int*> index;
  int* read_index_for_writing(std::size_t i) { return const_cast<int*>(index[i]); }
};

Good things about this solution: it works, and is safe, in every version of the standard and every compliant implementation. And it returns a vector reference with no funny wrapper classes – which probably doesn't matter to the caller, but it might.

Bad: you have to use the helper method internally, though only when reading the index for the purpose of writing the data. And the commenter described it as "dirty", but it seems clean enough to me.

benrg
  • 1,395
  • 11
  • 13
  • How is that safe? `vector` will construct a bunch of `const int*` objects. Modifying a const object though a non-const pointer is undefined behavior in the standard. – yeputons Apr 27 '22 at 20:02
  • I believe the last one is undefined: `*read_index_for_writing(k) = v;` – yeputons Apr 27 '22 at 20:10
  • Ah, never mind, I missed the part where `index` always points to `primaryData`. I'm sorry, perfectly safe in this case and no undefined behavior here (unless `primaryData` reallocates, but that's another story). – yeputons Apr 27 '22 at 20:40
2

Prepare the type like following, and use as return type of Data::getIndex().

class ConstIndex
{
private:
  const std::vector<int*> &index;
public:
  ConstIndex( const std::vector<int*> &index ) : index(index) {}

public:
  //Implement methods/types needed to emulate "const std::vector<const int*>"
  const int *operator[]( size_t i ) const { return index[i];    }
  const int *at( size_t i ) const { return index.at(i); }
  ...
};
fana
  • 1,370
  • 2
  • 7
  • `std::span` is very similar. – HolyBlackCat Apr 27 '22 at 06:58
  • This answer is just for the performance problem of "copy" written in this question. For the case of the sample written in this question, I will not employ this. I think that, the data structure inside the `Data` should be known only inside the `Data`, and it is not good that the user(outside) has to know it. Internal circumstances such as "How do you get the access to the data you want?" should be hidden (should be solved automatically/unknowingly). – fana Apr 28 '22 at 02:13
0

Here is an ugly solution that works with versions before C++20 using reinterpret_cast:

const std::vector<const int*>& getIndex() const{ 
    return reinterpret_cast<const std::vector<const int*>&>(data); 
}

Note this does actually return a reference bound to an lvalue, not a const& bound to an rvalue:

std::vector<const int*>& getIndex() const{ 
    return reinterpret_cast<std::vector<const int*>&>(data); 
}
Captain Hatteras
  • 490
  • 3
  • 11
  • 1
    Why doesn't the first cast cause UB? I don't think there's any guarantee that the memory layout must be the same for a container for T and const T. – Voo Apr 26 '22 at 11:56
  • It is UB. There's no reason to unsafely use `reinterpret_cast` when you can safely use `const_cast` – see [my answer](/a/72034512). – benrg Apr 27 '22 at 19:51