Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove non-essential null-checking from gsl::not_null #1051

Closed
dmitrykobets-msft opened this issue Aug 8, 2022 · 1 comment · Fixed by #1067
Closed

Remove non-essential null-checking from gsl::not_null #1051

dmitrykobets-msft opened this issue Aug 8, 2022 · 1 comment · Fixed by #1067

Comments

@dmitrykobets-msft
Copy link
Member

dmitrykobets-msft commented Aug 8, 2022

We are thinking of revising the gsl::not_null threat-model to reserve all nullness checking to the constructor, and to trust all created instances post-construction:

not_null<int*> a = some_pointer; // check that some_pointer is not null in a's constructor
int* b = a.get(); // no need to check that `a.ptr_` is not null; it's already been checked in a's constructor
not_null<int*> other = a; // no need to check that `a.ptr_` is not null; it's already been checked in a's constructor.  
                          // transitively, `other._ptr_` will now also be trusted

This change will remove many existing runtime checks that occur inside gsl::not_null, notably the one inside not_null::get(), and will overall simplify the design of the gsl::not_null type.

@KindDragon
Copy link

KindDragon commented Dec 12, 2022

I think the post at the link is relevant to this issue #930 (comment)

@dmitrykobets-msft dmitrykobets-msft assigned hsutter and unassigned hsutter Dec 13, 2022
dmitrykobets-msft added a commit that referenced this issue Dec 15, 2022
Guidelines issue 2006 removes the null check inside not_null::get, since the contained pointer is already guaranteed to be not-null upon construction.

Resolves #1051
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants