Pointers and the C++ Core Guidelines

[hpesoj]

First of all, thank you to everyone involved in the development of the C++ Core Guidelines and the GSL. Your work is invaluable and will undoubtedly be appreciated by generations of C++ programmers to come. With that in mind, I am concerned about the advice in the guidelines regarding the use of pointers, and have written a document outlining my thoughts, which can be summarised as:

• Using T* to represent "single objects" in high-level C++ code is less than ideal
• We can use the type system to enforce correctness at compile-time and document intent in-code
• There may be value in slightly rethinking the approach to annotations such as owner<T>

This document has been some time in the works, and has undergone many revisions, but it is now in a state that I am happy to share here. I would very much appreciate your consideration of my ideas, and welcome any comments or feedback. I believe it to be of utmost importance that the advice laid out in the guidelines is the best it can be. Please follow the link below to find the document with accompanying source code and unit tests.

[GitHub] Pointers and the C++ Core Guidelines

Joseph

[jwakely]

The expression p + n has undefined behaviour (for n != 0) if p points to a single object.

That's not true, p + 1 is valid too. So one of your central premises is incorrect.

For example, pointer arithmetic operations are always wrong if T* points to a single object.

Well you already said that p+n is OK for n==0, and it's also OK for n==1.
That allows you to use a single object as an array of length one:

int i = 0;
std::copy(&i, &i + 1, output);

Your proposal seems to be "remove all raw pointers". I don't think that's necessary, or realistic. When raw pointers are no longer used for the owning and array cases (as the guidelines recommend) then raw pointers do serve well for the remaining cases.

[hpesoj]

That's not true, p + 1 is valid too. So one of your central premises is incorrect.

Thanks. I guess I must have misinterpreted the standard. But this isn't really a central premise of my argument. It still doesn't make a great deal of sense for a type which is supposed to represent a "single object" to support pointer arithmetic. As the guidelines note, pointer arithmetic is tricky, so why should it be supported for "single objects"?

Your proposal seems to be "remove all raw pointers". I don't think that's necessary, or realistic. When raw pointers are no longer used for the owning and array cases (as the guidelines recommend) then raw pointers do serve well for the remaining cases.

My proposal isn't to remove all raw pointers; it's to provide strongly typed, recommended alternatives that do a much better job. Raw pointers will still exist in low-level and legacy code, and I mention later in the proposal that I do recommend that a bare T* represents a "single object" (and be implicitly "not null"). Thus, under my proposal, static analysis tools will not flag T* when used as a "not null single object". I am simply proposing preferred alternatives, the tangible benefits (beyond prohibiting pointer arithmetic) of which I outline in detail in the proposal.

EDIT

I have corrected the incorrect information about pointer arithmetic, and added a disclaimer about the aim of the proposal.

[BjarneStroustrup]

Our strategy in the core guidelines is to limit raw pointers to situations where they are harmless or unavoidable (e.g., string literals). We see no hope of eliminating all raw pointers in the billions of lines of C++ "out there". We do hope that we can provide a path for gradual elimination of unsafe and/or tricky uses. We know that at best such elimination will take many years.

[hpesoj]

Our strategy in the core guidelines is to limit raw pointers to situations where they are harmless or unavoidable (e.g., string literals). We see no hope of eliminating all raw pointers in the billions of lines of C++ "out there". We do hope that we can provide a path for gradual elimination of unsafe and/or tricky uses. We know that at best such elimination will take many years.

Do you aim to draw a distinction between what is "acceptable" and what is recommended? For example, the optional_ref<T> class (which is essentially an implementation of std::optional<T&>) is safer, clearer and more convenient than T* when used as an "optional reference" parameter (rule F.60). So while T* could be considered "okay" by the guidelines, especially in old code where such use is prevalent, optional_ref<T> could be the recommended approach, particularly for new code.

If the guidelines are focusing on making old C++ code safer rather than on best practices for new code, maybe I need to propose these things for standardization instead?

[hsutter]

Thanks for your interest and support. However, we do fundamentally believe that T* should point to a single object,

C did not distinguish in the type system between pointer to single objects and pointers used as array iterators, so we need to give a distinct name to each to distinguish them:

• Pointers to single objects are useful and widely used; they represent the large majority of current uses of pointers. For that reason, they should not be given a new name, whether your observer<T> or, as I think people mentioned to you also on the Boost lists, the observer_ptr<T> currently being standardized as another attempt in a similar direction. We don't think it's a fruitful path to try to give a new name to this when it already has a widespread and established spelling, T*.
• Pointers used as array iterators represent a smaller set of current uses of T* and using T* for this is inherently brittle and bounds-unsafe. Therefore these are the ones that should get a new name -- and we believe that this GSL's span<T> (now being actively standardized) is the correct bounds-safe way to refer to arrays of objects.

Additionally, we are actively writing a formalization of the Lifetime rules you can find in the /docs directory that does static dangling prevention. It's a general approach that is designed to work equally with dangling pointers and dangling spans and invalidated iterators, with the same set of rules and without special handling for any of those types.

We believe this approach is the best and most compatible for C++ code, and we're committed to following this direction. But we thank you for your interest and support; the Guidelines are definitely trying to tackle big problems, including this one. Thanks!

[hpesoj]

Hi Herb Sutter. Thanks for taking the time to consider my suggestions and respond.

Thanks for your interest and support. However, we do fundamentally believe that T* should point to a single object,

So do I. in fact, I would take this interface reduction one step further and make an un-annotated T* implicitly "not null". I just believe there is additional value in providing statically type-safe recommended alternatives for each of the remaining uses of T* in high-level code.

Pointers to single objects are useful and widely used; they represent the large majority of current uses of pointers. For that reason, they should not be given a new name, whether your observer<T> or, as I think people mentioned to you also on the Boost lists, the observer_ptr<T> currently being standardized as another attempt in a similar direction. We don't think it's a fruitful path to try to give a new name to this when it already has a widespread and established spelling, T*.

observer<T> (or whatever the most appropriate name for it is) is not a new name for a T* that represents a non-owning single object. It is a new name for a non-owning single object that is retained (i.e. a copy of it persists in object or global state), while T& represents a non-retained single object. It is useful to distinguish between retained and non-retained because it allows intent to be documented in-code. This goes beyond preventing dangling pointers, because it allows programmers to more easily reason about their code. If I see observer<T> or T&, I know immediately exactly what is going on. Conversely, if I see T*, I am forced to think, and not_null<T*> doesn't make things much better.

By designating T& as non-retained and observer<T> as retained, and extending optional<T> to support references, the guidelines for "mandatory vs. optional" and "retained vs. not retained" can be decomposed into two orthogonal sets of rules:

We believe this approach is the best and most compatible for C++ code, and we're committed to following this direction. But we thank you for your interest and support; the Guidelines are definitely trying to tackle big problems, including this one. Thanks!

I admit I am slightly puzzled by this mindset. What I'm hearing is, "Yes, T* is not perfect, and yes, its use violates a number of the guidelines' own rules about type-safety and documentation of intent, but since it is already widely used in old C++ code, and we feel that it isn't all that bad, we may as well recommend its widespread use in new C++ code and try to patch over the cracks with static analysis." I thought that the guidelines were meant to be "best practices", not "good enough practices".

Just to be clear, I don't think it is reasonable to expect all uses of T* to be replaced in all code. The "good enough" attitude is actually reasonable for old code, because it simply may not be worth the effort to replace every single instance of T*. However, in new C++ code, I see no reason to continue to recommend use of T* if safer, clearer alternatives can be recommended. I just think, if I were browsing an unfamiliar code base, which of these would I rather encounter?

void foo(bar* b);           // This?
void foo(optional<bar&> b); // Or this?

set_widget(make_observer(widget)); // This?
set_widget(&widget);               // Or this?

I hope I'm not labouring the point. It's just I feel that these are valid concerns that are being dismissed because of a failure to distinguish between best practices for new code, and pragmatic recommendations for updating old code.

[hsutter]

Thanks. Speaking to the first point:

I would take this interface reduction one step further and make an un-annotated T* implicitly "not null".

I understand, and we considered that.

We decided against that for several reasons:

• T*, smart_ptr<T>, span<T>, container<T>::iterator, range<T>, etc. are all non-owning indirections and should be consistent with each other -- it would be strange for some to be nullable but not others. Iterators can be "null", for example a default-constructed iterator is not referring to anything.
• More generally, all of those can be default-constructed, and the only reasonable semantics for that are "doesn't point to anything." (This can be a springboard for a broader discussion about the situations where default-constructible types are important, Regular types, etc.)
• A large fraction of existing of T* are deliberately intended to be null, because people by convention use references for not-null parameters in particular and so in modern C++ code the presence of a T* parameter often (not always) implies nullability by that convention. So trying to annotate the "nullable" case is a huge code churn, and not only unadoptable but actually against the intent of much existing code.
• Even if we ignored that and changed the default for T*, then we'd need to invent yet another annotation wrapper such as nullable<T>, and have to teach and explain both not_null<T> and nullable<T> (inconsistently).

For these and other reasons, we think that pointers should be nullable by default unless annotated otherwise.

valid concerns that are being dismissed because of a failure to distinguish between best practices for new code, and pragmatic recommendations for updating old code

I hope that helps reassure you that the concerns were considered deeply and aren't being dismissed, and apply both to new code and old code. Defaults are important, and should reflect the common case especially for new code, but also for old code much of which is "correct" but just expressed without enough information about the intent because the programmer didn't have the option or tool to express the intent.

The key issue is to distinguish maybe-null and never-null in the type system, and both of our approaches agree on doing that. Tony Hoare called null pointers his "billion-dollar mistake," but in my opinion, and I think yours, the mistake was not maybe-null pointers (which are necessary, unavoidable, and pervasively present in every language with pointer/reference indirections, including Java, C#, C, C++, etc.), but rather in not distinguishing maybe-null and never-null pointers in the type system. You and we are both trying to do that, and so in the above I think we're largely agreeing and our discussion is narrowly just about which one should be the default.

[hsutter]

And in case I wasn't clear enough, I do think "nullable" is the correct default in brand-new clean code as the common case, even ignoring legacy issues. But to make it tool-enforceable, we to have enable programmers to state their intent of whether a given pointer variable is intended to be nullable or not; then we can write useful noise-free tools that ensure the code does the null check on all-and-only those pointers, which we believe can and will prevent null-deref bugs. Hence the importance of distinguishing it in the type to express the intent.

[hpesoj]

You make a fair case for T* being "nullable" by default. Regularity is a useful property that a lot of code takes advantage of. It would probably be an uphill battle to try and suppress the regularity of T* without using some kind of adapter type (e.g. not_null<T>).

I hope that helps reassure you that the concerns were considered deeply and aren't being dismissed, and apply both to new code and old code. Defaults are important, and should reflect the common case especially for new code, but also for old code much of which is "correct" but just expressed without enough information about the intent because the programmer didn't have the option or tool to express the intent.

Whether or not T* should be considered "nullable" by default was one of my lesser concerns (though you have changed my mind). My major concern is that use of T* is being recommended even for new code despite its shortcomings. You say that you agree that it was a mistake to not distinguish between maybe-null and never-null in the type system. Come C++17, we will have an idiomatic way to express the concept of "maybe null" using the standard library: optional<T>. My proposal is to provide the means for optional<T> to be used with "non-owning single-object indirections" by:

1. Defining optional<T&> as an "optional" indirection type with value semantics
2. Providing observer<T> as a statically type-safe "not null" indirection type with reference semantics

Note: The naming of observer can be bike-shedded ad-nauseum. I'm sticking with observer for demonstrative purposes.

It is natural to provide different types for "retained" and "not retained" indirections. If you aren't retaining an indirection, chances are you only care about the value of the referenced object, so it makes sense to use T&, a type with value semantics (it behaves like T). On the other hand, if you are retaining an indirection, chances are you care about the identity of the referenced object, so it makes sense to use observer<T>, a type with reference semantics (it behaves like T*).

The (overlapping) concerns about T* that I feel are being dismissed are:

• Its lack of static type-safety as a "not null" type (not_null<T> only provides run-time safety)
• Its lack of type-safety in general (T* is used for many things)
• Its inability to document intent (retained vs. not retained)
• Its incorrect semantics when used as an "optional" T& (cannot pass temporaries; cannot be compared with T)

There is also the concern that use of T* violates the guidelines' own rules about static type-safety and clear and explicit documentation of intent.

[hsutter]

Whether or not T* should be considered "nullable" by default was one of my lesser concerns (though you have changed my mind).

Thanks. I responded to it because it was your first point, and you'd put a lot of thought into it and I felt it deserved a more detailed response than "yes we thought of all those things." I didn't have cycles to respond in equal detail to everything, so I picked the first, but the other points were considered too.

Thanks for summarizing your major concerns, I'll try to give one more set of responses to those:

The (overlapping) concerns about T* that I feel are being dismissed are:

• Its lack of static type-safety as a "not null" type (not_null only provides run-time safety)

Assuming by type-safety you mean not dereferencing null: The Lifetime rules I referred to include adding some static safety for not_null, including that not_nulls must be checked against null before being dereferenced. (The level of safety guarantee we can formally state is tbd as we do the formal specification.)

• Its lack of type-safety in general (T* is used for many things)

We agree that the T* type is currently used for multiple things that should be distinguished in the type, and we do:

• To point to an array and do array iteration, use span<T>. That removes the main type-unsafe use of T*, and using T* for this is now banned in the Guidelines.
• To point to a single object and never null, use not_null<T*>.
• To point to a single object or null, use T*. This is what most T*s are already used for and we can and should continue to use that name.
  Orthogonally, the Lifetime rules statically prevent dangling for all of the above (and more, such as iterators).

• Its inability to document intent (retained vs. not retained)

This is also covered in the Lifetime static rules.

• If it's an owner, nothing needs to be done, it's expressed in the type (e.g., shared_ptr<T>).
• If it's a non-owner, we will permit (optional) annotation when doing "unusual" lifetime manipulation of non-owning indirections. Note the general terms:
  • "Unusual lifetime manipulation" is anything where we can't statically know that the lifetime is safe from dangling. This covers "retain" semantics, but also much more.
  • "Non-owning indirections" is the general term used in the Lifetime paper to equally apply the rules to T*, span<T>, container<T>::iterator, range<T>, and other non-owners that could dangle today without the static rules. We don't want to have special rules that work for pointers, but not for iterators or spans or ranges for example. (I realize you also propose an iterator<T>, though that seems to be more a replacement for span<T>, which I don't think we need esp. since span<T> is already being standardized.)

So we prefer general rules that work for more than just pointers, and work for more than just "retain" semantics, to express non-owning lifetime safely.

• Its incorrect semantics when used as an "optional" T& (cannot pass temporaries; cannot be compared with T)

Cannot compare unless you dereference to compare *pt with t of course. But if you prefer references, those also are still fine and encouraged for parameters, and the same lifetime rules that apply to pointers also apply to references (a T& is essentially just T*const plus some syntactic sugar for callers).

Even if you don't agree with our conclusions on some of these, at least please be assured that they're not being dismissed. We did consider them and are aware of these issues, as well as more general ones as we consider also things like iterator invalidation issues as well as we aim for a single consistent model.

[elronayellin]

To point to a single object or null, use T*. This is what most T*s are already used for and we can and should continue to use that name.

If I encounter a T* I don’t know if the original author meant T* in the sense of these guidelines or not, so I need to look carefully at the code and decide if it should be changed to not_null<T*> or left alone (for the next person to ask the same question). I’d rather be explicit and use maybe_null<T*> and not_null<T*>. Then when I see T*, I know it’s older code or code written by someone not following these guidelines, and I can then determine how to modernize it.

[hpesoj]

Thanks. I responded to it because it was your first point, and you'd put a lot of thought into it and I felt it deserved a more detailed response than "yes we thought of all those things."

Thanks, and I appreciate it. I didn't mean to make it sound like I didn't appreciate your response. Despite being the lesser of my concerns, I wouldn't have mentioned it if I didn't want to discuss it.

Assuming by type-safety you mean not dereferencing null:

By "type-safety", I am referring to the aforementioned inability of C++ to distinguish between never-null and maybe-null pointers. But yeah, it comes down to preventing null pointer dereferencing.

The Lifetime rules I referred to include adding some static safety for not_null, including that not_nulls must be checked against null before being dereferenced. (The level of safety guarantee we can formally state is tbd as we do the formal specification.)

I assume you meant that T*s must be checked against null before dereferencing? observer<T> enforces the "not null" condition using the compiler, and will work even if static analysis fails, or if there is no static analysis at all. Do you think there is any value in this?

I realize you also propose an iterator<T>, though that seems to be more a replacement for span<T>, which I don't think we need esp. since span<T> is already being standardized.

iterator<T> is a simple templated type alias, like owner<T>. My suggestion still stands, just change nullable<T> to not_null<T>:

template <typename T> using array = T;
template <typename T> using iterator = T;
template <typename T> using not_null = T;
template <typename T> using owner = T;

We don't want to have special rules that work for pointers, but not for iterators or spans or ranges for example.

So we prefer general rules that work for more than just pointers, and work for more than just "retain" semantics, to express non-owning lifetime safely.

What I am proposing can be checked under the lifetime rules as they stand. observer<T> provides no more lifetime checking ability than T&. I say that observer<T> is conceptually a "retained" pointer because that is the primary capacity in which I see it being used. There is no obligation to use it this way, just as there is no obligation to not retain a T& (though I would recommend against it). observer<T> would require no special treatment by the lifetime rules (unlike not_null<T>).

Cannot compare unless you dereference to compare *pt with t of course. But if you prefer references, those also are still fine and encouraged for parameters, and the same lifetime rules that apply to pointers also apply to references (a T& is essentially just T*const plus some syntactic sugar for callers).

I cannot use T& for optional parameters. Here is an example demonstrating my two points:

auto f = [](vec3 const* v) {
    if (v && *v == vec3()) { // no direct comparison
        …
    }
};

auto g = [](optional<vec3 const&> v) {
    if (v == vec3()) { // direct comparison
        …
    }
};

f(&vec3(1, 2, 3)); // error: cannot take address of rvalue
g(vec3(1, 2, 3)); // fine

Note that I am proposing the addition of optional_ref<T> into the GSL, with a view to getting optional<T&> into C++20 (given that three years is a long time, I see value in adding optional_ref so that the guidelines can be ahead of the curve).

Even if you don't agree with our conclusions on some of these, at least please be assured that they're not being dismissed. We did consider them and are aware of these issues, as well as more general ones as we consider also things like iterator invalidation issues as well as we aim for a single consistent model.

Thanks. I'm glad you are considering these things. However, we clearly don't see eye to eye. My view is that T* and not_null<T*> are not well-suited for use in modern, high-level C++ code as "single objects". T& and observer<T> coupled with optional<T> seems like a far superior solution that enforces correctness using the compiler as well as conveying intent to the programmer. Static analysis tools are invaluable for detecting a whole range of issues that are not caught by the compiler, but I don't think static analysis is a substitute for good interface design.

[rob-stewart]

I'm not sure about the naming everything, but I see a lot of sense in using specific types for everything. Then, as elronayellin noted, T * in code signals legacy usage rather than the GSL-accepted usage.