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

Checks in not_null. #604

Closed
john-preston opened this issue Jan 23, 2018 · 2 comments
Closed

Checks in not_null. #604

john-preston opened this issue Jan 23, 2018 · 2 comments
Assignees

Comments

@john-preston
Copy link

Current implementation of not_null does checks (by Expects and Ensures):

  • in every constructor (even if constructed from other not_null)
  • in every dereference using any method (all go through get())

This could be an overhead in case you use not_null quite a lot and pass it everywhere by value and leave the Expects()/Ensures() checks even in the Release build (my case). Is this intended or should the check really be only in raw-pointer constructor?

I don't see any sense in those additional checks for not_null<raw_pointer> types, because they can't suddenly become null in the middle of their lifetime. If this is not intended I can prepare a PR with single check. It also should fix #535 because currently copy constructor is not used for non-const_ref not_null copies.

@xaxxon
Copy link
Contributor

xaxxon commented Apr 13, 2018

This is addressed in this PR #650 by not checking nullness in get.

@beinhaerter
Copy link
Contributor

Is there a reason to keep this open? As far as I can see it is solved.

    not_null(const not_null& other) = default;
    constexpr details::value_or_reference_return_t<T> get() const { return ptr_; }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants