GeezerVison

Do we need the functionality?

If ever we want to use these classes with any generic function/algorithm(ie swap) then we do. These are fundamental operations on types.

Making classes non-regular very much limits what we can do in the future. There is already certain types of analysis we can't do because TokenList and Token are not regular classes.

If ever we want to use these classes with any generic function/algorithm(ie swap) then we do. These are fundamental operations on types.

Ok. Can we leave the parameter a reference and just store the pointer?

Making classes non-regular very much limits what we can do in the future. There is already certain types of analysis we can't do because TokenList and Token are not regular classes.

Could you please track those issues in tickets? Thanks.

The constParameter false negatives exposed by these changes are being tracked in https://trac.cppcheck.net/ticket/11599.

Ok. Can we leave the parameter a reference and just store the pointer?

Yes, parameters should be references, but member variables should not.

Ok. Can we leave the parameter a reference and just store the pointer?

Yes, parameters should be references, but member variables should not.

And if I hand it out I could make it a reference again?

That sounds like you should have something like a std::shared_ptrwhich doesn't allow nullptr but that could wreck havoc with the lifetime of objects which isn't good either.

We want something like gsl::non_null.

Sometimes I am just stupid - what about #4785? I was thinking about disallowing nullptr in that.

We already use this pattern all over the place and it is not an isolated case. So could we discuss this in the other ticket and merge this? I have a few more commits depending on this and IIRC those are just parameter and not member changes so things and even if things would just very, very slightly get worse.

Sometimes I am just stupid - what about #4785?

I dont think its helpful to have a non-owning pointer propagate const. It can be simply bypassed by copying the pointer:

void f(const safe_ptr<int>& ptr) {
    // *ptr += 1 doesnt compile, but the below does
    auto ptr2 = ptr;
    *ptr += 1;
}

Its better to use const int*, which will prevent mutations to the data(and it can be used as a member variable).

I dont think its helpful to have a non-owning pointer propagate const. It can be simply bypassed by copying the pointer:

Obviously - but those are not meant to be used outside of the class. It's to enforce the "physical constness" as far as possible and reducing the code which relies on "logical constness".

Its better to use const int*, which will prevent mutations to the data(and it can be used as a member variable).

If it is const T * to begin with there is no need for safe_ptr. This is for cases where it has to be mutable within the class but we don't want it to be mutable within const methods.

Another constParameter false negative exposed by this is already being tracked as https://trac.cppcheck.net/ticket/11097.