Simple device (fancy) pointer implementation The 2019 Stack Overflow Developer Survey Results Are In Announcing the arrival of Valued Associate #679: Cesar Manara Planned maintenance scheduled April 17/18, 2019 at 00:00UTC (8:00pm US/Eastern)Trying to find a good design for reading in values of different types from a fileUnique pointer implementationHeap implementation using pointerSimple shared pointerArray-like container for uints shorter than 8 bits (Rev 1)Alternate “weak” pointer implementationPreferred implementation of `Array<T>::operator=(const Array<T> & rhs)`C++ maybe pointer type implementationAttempt at Smart Pointer ImplementationC++: Smart Pointer Implementation
What can I do if neighbor is blocking my solar panels intentionally?
First use of “packing” as in carrying a gun
How to stretch delimiters to envolve matrices inside of a kbordermatrix?
Make it rain characters
What's the point in a preamp?
Was credit for the black hole image misattributed?
Do warforged have souls?
Python - Fishing Simulator
Can a 1st-level character have an ability score above 18?
Can the prologue be the backstory of your main character?
I could not break this equation. Please help me
how can a perfect fourth interval be considered either consonant or dissonant?
What do you call a plan that's an alternative plan in case your initial plan fails?
Am I ethically obligated to go into work on an off day if the reason is sudden?
University's motivation for having tenure-track positions
Are my PIs rude or am I just being too sensitive?
Mortgage adviser recommends a longer term than necessary combined with overpayments
How can I define good in a religion that claims no moral authority?
What information about me do stores get via my credit card?
Why is superheterodyning better than direct conversion?
Is every episode of "Where are my Pants?" identical?
How many people can fit inside Mordenkainen's Magnificent Mansion?
Who or what is the being for whom Being is a question for Heidegger?
How is simplicity better than precision and clarity in prose?
Simple device (fancy) pointer implementation
The 2019 Stack Overflow Developer Survey Results Are In
Announcing the arrival of Valued Associate #679: Cesar Manara
Planned maintenance scheduled April 17/18, 2019 at 00:00UTC (8:00pm US/Eastern)Trying to find a good design for reading in values of different types from a fileUnique pointer implementationHeap implementation using pointerSimple shared pointerArray-like container for uints shorter than 8 bits (Rev 1)Alternate “weak” pointer implementationPreferred implementation of `Array<T>::operator=(const Array<T> & rhs)`C++ maybe pointer type implementationAttempt at Smart Pointer ImplementationC++: Smart Pointer Implementation
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;
$begingroup$
device_raw_ptr
is a simple fancy pointer. It essentially wrap pointers to GPU memory. It's sole purpose is to separate out host pointers from device pointers, i.e. they should not be inter-convertible and must not be dereferenced. At the same time, they should be zero-cost (with respect to raw host pointers) and be maximally compatible with regular pointers.
Helper class:
template <class T>
struct equality_operators
/*
** The deriving class must implement the following:
** friend bool operator==(const T&, const T&);
*/
friend bool operator!=(const T& lhs, const T& rhs) return !static_cast<bool>(lhs == rhs);
;
template <class T>
struct less_than_operators
/*
** The deriving class must implement the following:
** friend bool operator<(const T&, const T&);
*/
friend bool operator>(const T& lhs, const T& rhs) return rhs < lhs;
friend bool operator<=(const T& lhs, const T& rhs) return !static_cast<bool>(lhs > rhs);
friend bool operator>=(const T& lhs, const T& rhs) return !static_cast<bool>(lhs < rhs);
;
template <class T>
struct relational_operators : equality_operators<T>, less_than_operators<T> ;
device_raw_ptr
implementation:
template <class T>
class device_raw_ptr : public relational_operators<T>
static_assert(std::is_standard_layout<T>::value, "T must satisfy StandardLayoutType");
public:
using element_type = std::remove_extent_t<T>;
constexpr device_raw_ptr() noexcept = default;
constexpr device_raw_ptr(std::nullptr_t) noexcept : ptr nullptr
constexpr device_raw_ptr(const device_raw_ptr& other) noexcept = default;
explicit device_raw_ptr(element_type* ptr_) noexcept : ptr ptr_
device_raw_ptr(device_raw_ptr&& other) noexcept : ptr other.ptr other.reset();
device_raw_ptr& operator=(const device_raw_ptr& other) noexcept
swap(device_raw_ptr(other), *this);
return *this;
device_raw_ptr& operator=(device_raw_ptr&& other) noexcept
swap(device_raw_ptr(other), *this);
return *this;
void reset() noexcept ptr = nullptr;
void reset(T* ptr_) noexcept ptr = ptr_;
element_type* get() noexcept return ptr; ;
const element_type* get() const noexcept return ptr;
friend void swap(device_raw_ptr& lhs, device_raw_ptr& rhs) noexcept
using std::swap;
std::swap(lhs.ptr, rhs.ptr);
explicit operator bool() const noexcept return static_cast<bool>(ptr);
device_raw_ptr& operator++() noexcept
++ptr;
return *this;
device_raw_ptr operator++(int) noexcept
device_raw_ptr tmp(*this);
ptr++;
return tmp;
device_raw_ptr& operator+=(std::ptrdiff_t offset) noexcept
ptr += offset;
return *this;
device_raw_ptr& operator-=(std::ptrdiff_t offset) noexcept
ptr -= offset;
return *this;
friend device_raw_ptr& operator+(device_raw_ptr lhs, std::ptrdiff_t offset) noexcept
lhs += offset;
return lhs;
friend device_raw_ptr& operator-(device_raw_ptr lhs, std::ptrdiff_t offset) noexcept
lhs -= offset;
return lhs;
/* required by relational_operators base class */
friend bool operator==(const device_raw_ptr& lhs, const device_raw_ptr& rhs) noexcept return lhs.ptr == rhs.ptr;
friend bool operator<(const device_raw_ptr& lhs, const device_raw_ptr& rhs) noexcept return lhs.ptr < rhs.ptr;
protected:
T *ptr;
;
template <class T, class U, class V>
std::basic_ostream<U, V>& operator<<(std::basic_ostream<U, V>& os, const device_raw_ptr<T>& other)
os << other.get() << " (device)";
return os;
I am also looking for suggestions on how to order different things inside a class.
Might it be better to initialize with nullptr
instead of the default constructor. This breaks compatibility but I think it might be worth considering the compiler would mostly optimize the assignment if it's immediately overwritten.
c++ c++11 pointers
New contributor
$endgroup$
add a comment |
$begingroup$
device_raw_ptr
is a simple fancy pointer. It essentially wrap pointers to GPU memory. It's sole purpose is to separate out host pointers from device pointers, i.e. they should not be inter-convertible and must not be dereferenced. At the same time, they should be zero-cost (with respect to raw host pointers) and be maximally compatible with regular pointers.
Helper class:
template <class T>
struct equality_operators
/*
** The deriving class must implement the following:
** friend bool operator==(const T&, const T&);
*/
friend bool operator!=(const T& lhs, const T& rhs) return !static_cast<bool>(lhs == rhs);
;
template <class T>
struct less_than_operators
/*
** The deriving class must implement the following:
** friend bool operator<(const T&, const T&);
*/
friend bool operator>(const T& lhs, const T& rhs) return rhs < lhs;
friend bool operator<=(const T& lhs, const T& rhs) return !static_cast<bool>(lhs > rhs);
friend bool operator>=(const T& lhs, const T& rhs) return !static_cast<bool>(lhs < rhs);
;
template <class T>
struct relational_operators : equality_operators<T>, less_than_operators<T> ;
device_raw_ptr
implementation:
template <class T>
class device_raw_ptr : public relational_operators<T>
static_assert(std::is_standard_layout<T>::value, "T must satisfy StandardLayoutType");
public:
using element_type = std::remove_extent_t<T>;
constexpr device_raw_ptr() noexcept = default;
constexpr device_raw_ptr(std::nullptr_t) noexcept : ptr nullptr
constexpr device_raw_ptr(const device_raw_ptr& other) noexcept = default;
explicit device_raw_ptr(element_type* ptr_) noexcept : ptr ptr_
device_raw_ptr(device_raw_ptr&& other) noexcept : ptr other.ptr other.reset();
device_raw_ptr& operator=(const device_raw_ptr& other) noexcept
swap(device_raw_ptr(other), *this);
return *this;
device_raw_ptr& operator=(device_raw_ptr&& other) noexcept
swap(device_raw_ptr(other), *this);
return *this;
void reset() noexcept ptr = nullptr;
void reset(T* ptr_) noexcept ptr = ptr_;
element_type* get() noexcept return ptr; ;
const element_type* get() const noexcept return ptr;
friend void swap(device_raw_ptr& lhs, device_raw_ptr& rhs) noexcept
using std::swap;
std::swap(lhs.ptr, rhs.ptr);
explicit operator bool() const noexcept return static_cast<bool>(ptr);
device_raw_ptr& operator++() noexcept
++ptr;
return *this;
device_raw_ptr operator++(int) noexcept
device_raw_ptr tmp(*this);
ptr++;
return tmp;
device_raw_ptr& operator+=(std::ptrdiff_t offset) noexcept
ptr += offset;
return *this;
device_raw_ptr& operator-=(std::ptrdiff_t offset) noexcept
ptr -= offset;
return *this;
friend device_raw_ptr& operator+(device_raw_ptr lhs, std::ptrdiff_t offset) noexcept
lhs += offset;
return lhs;
friend device_raw_ptr& operator-(device_raw_ptr lhs, std::ptrdiff_t offset) noexcept
lhs -= offset;
return lhs;
/* required by relational_operators base class */
friend bool operator==(const device_raw_ptr& lhs, const device_raw_ptr& rhs) noexcept return lhs.ptr == rhs.ptr;
friend bool operator<(const device_raw_ptr& lhs, const device_raw_ptr& rhs) noexcept return lhs.ptr < rhs.ptr;
protected:
T *ptr;
;
template <class T, class U, class V>
std::basic_ostream<U, V>& operator<<(std::basic_ostream<U, V>& os, const device_raw_ptr<T>& other)
os << other.get() << " (device)";
return os;
I am also looking for suggestions on how to order different things inside a class.
Might it be better to initialize with nullptr
instead of the default constructor. This breaks compatibility but I think it might be worth considering the compiler would mostly optimize the assignment if it's immediately overwritten.
c++ c++11 pointers
New contributor
$endgroup$
$begingroup$
I just noticed that I am passing rvalues toswap
which accepts non-const lvalue as arguments. You can find it in the copy & move constructor.
$endgroup$
– Yashas
Apr 7 at 16:56
$begingroup$
If anyone did not notice, I messed up therelational_operator
inheritance.
$endgroup$
– Yashas
Apr 8 at 5:14
add a comment |
$begingroup$
device_raw_ptr
is a simple fancy pointer. It essentially wrap pointers to GPU memory. It's sole purpose is to separate out host pointers from device pointers, i.e. they should not be inter-convertible and must not be dereferenced. At the same time, they should be zero-cost (with respect to raw host pointers) and be maximally compatible with regular pointers.
Helper class:
template <class T>
struct equality_operators
/*
** The deriving class must implement the following:
** friend bool operator==(const T&, const T&);
*/
friend bool operator!=(const T& lhs, const T& rhs) return !static_cast<bool>(lhs == rhs);
;
template <class T>
struct less_than_operators
/*
** The deriving class must implement the following:
** friend bool operator<(const T&, const T&);
*/
friend bool operator>(const T& lhs, const T& rhs) return rhs < lhs;
friend bool operator<=(const T& lhs, const T& rhs) return !static_cast<bool>(lhs > rhs);
friend bool operator>=(const T& lhs, const T& rhs) return !static_cast<bool>(lhs < rhs);
;
template <class T>
struct relational_operators : equality_operators<T>, less_than_operators<T> ;
device_raw_ptr
implementation:
template <class T>
class device_raw_ptr : public relational_operators<T>
static_assert(std::is_standard_layout<T>::value, "T must satisfy StandardLayoutType");
public:
using element_type = std::remove_extent_t<T>;
constexpr device_raw_ptr() noexcept = default;
constexpr device_raw_ptr(std::nullptr_t) noexcept : ptr nullptr
constexpr device_raw_ptr(const device_raw_ptr& other) noexcept = default;
explicit device_raw_ptr(element_type* ptr_) noexcept : ptr ptr_
device_raw_ptr(device_raw_ptr&& other) noexcept : ptr other.ptr other.reset();
device_raw_ptr& operator=(const device_raw_ptr& other) noexcept
swap(device_raw_ptr(other), *this);
return *this;
device_raw_ptr& operator=(device_raw_ptr&& other) noexcept
swap(device_raw_ptr(other), *this);
return *this;
void reset() noexcept ptr = nullptr;
void reset(T* ptr_) noexcept ptr = ptr_;
element_type* get() noexcept return ptr; ;
const element_type* get() const noexcept return ptr;
friend void swap(device_raw_ptr& lhs, device_raw_ptr& rhs) noexcept
using std::swap;
std::swap(lhs.ptr, rhs.ptr);
explicit operator bool() const noexcept return static_cast<bool>(ptr);
device_raw_ptr& operator++() noexcept
++ptr;
return *this;
device_raw_ptr operator++(int) noexcept
device_raw_ptr tmp(*this);
ptr++;
return tmp;
device_raw_ptr& operator+=(std::ptrdiff_t offset) noexcept
ptr += offset;
return *this;
device_raw_ptr& operator-=(std::ptrdiff_t offset) noexcept
ptr -= offset;
return *this;
friend device_raw_ptr& operator+(device_raw_ptr lhs, std::ptrdiff_t offset) noexcept
lhs += offset;
return lhs;
friend device_raw_ptr& operator-(device_raw_ptr lhs, std::ptrdiff_t offset) noexcept
lhs -= offset;
return lhs;
/* required by relational_operators base class */
friend bool operator==(const device_raw_ptr& lhs, const device_raw_ptr& rhs) noexcept return lhs.ptr == rhs.ptr;
friend bool operator<(const device_raw_ptr& lhs, const device_raw_ptr& rhs) noexcept return lhs.ptr < rhs.ptr;
protected:
T *ptr;
;
template <class T, class U, class V>
std::basic_ostream<U, V>& operator<<(std::basic_ostream<U, V>& os, const device_raw_ptr<T>& other)
os << other.get() << " (device)";
return os;
I am also looking for suggestions on how to order different things inside a class.
Might it be better to initialize with nullptr
instead of the default constructor. This breaks compatibility but I think it might be worth considering the compiler would mostly optimize the assignment if it's immediately overwritten.
c++ c++11 pointers
New contributor
$endgroup$
device_raw_ptr
is a simple fancy pointer. It essentially wrap pointers to GPU memory. It's sole purpose is to separate out host pointers from device pointers, i.e. they should not be inter-convertible and must not be dereferenced. At the same time, they should be zero-cost (with respect to raw host pointers) and be maximally compatible with regular pointers.
Helper class:
template <class T>
struct equality_operators
/*
** The deriving class must implement the following:
** friend bool operator==(const T&, const T&);
*/
friend bool operator!=(const T& lhs, const T& rhs) return !static_cast<bool>(lhs == rhs);
;
template <class T>
struct less_than_operators
/*
** The deriving class must implement the following:
** friend bool operator<(const T&, const T&);
*/
friend bool operator>(const T& lhs, const T& rhs) return rhs < lhs;
friend bool operator<=(const T& lhs, const T& rhs) return !static_cast<bool>(lhs > rhs);
friend bool operator>=(const T& lhs, const T& rhs) return !static_cast<bool>(lhs < rhs);
;
template <class T>
struct relational_operators : equality_operators<T>, less_than_operators<T> ;
device_raw_ptr
implementation:
template <class T>
class device_raw_ptr : public relational_operators<T>
static_assert(std::is_standard_layout<T>::value, "T must satisfy StandardLayoutType");
public:
using element_type = std::remove_extent_t<T>;
constexpr device_raw_ptr() noexcept = default;
constexpr device_raw_ptr(std::nullptr_t) noexcept : ptr nullptr
constexpr device_raw_ptr(const device_raw_ptr& other) noexcept = default;
explicit device_raw_ptr(element_type* ptr_) noexcept : ptr ptr_
device_raw_ptr(device_raw_ptr&& other) noexcept : ptr other.ptr other.reset();
device_raw_ptr& operator=(const device_raw_ptr& other) noexcept
swap(device_raw_ptr(other), *this);
return *this;
device_raw_ptr& operator=(device_raw_ptr&& other) noexcept
swap(device_raw_ptr(other), *this);
return *this;
void reset() noexcept ptr = nullptr;
void reset(T* ptr_) noexcept ptr = ptr_;
element_type* get() noexcept return ptr; ;
const element_type* get() const noexcept return ptr;
friend void swap(device_raw_ptr& lhs, device_raw_ptr& rhs) noexcept
using std::swap;
std::swap(lhs.ptr, rhs.ptr);
explicit operator bool() const noexcept return static_cast<bool>(ptr);
device_raw_ptr& operator++() noexcept
++ptr;
return *this;
device_raw_ptr operator++(int) noexcept
device_raw_ptr tmp(*this);
ptr++;
return tmp;
device_raw_ptr& operator+=(std::ptrdiff_t offset) noexcept
ptr += offset;
return *this;
device_raw_ptr& operator-=(std::ptrdiff_t offset) noexcept
ptr -= offset;
return *this;
friend device_raw_ptr& operator+(device_raw_ptr lhs, std::ptrdiff_t offset) noexcept
lhs += offset;
return lhs;
friend device_raw_ptr& operator-(device_raw_ptr lhs, std::ptrdiff_t offset) noexcept
lhs -= offset;
return lhs;
/* required by relational_operators base class */
friend bool operator==(const device_raw_ptr& lhs, const device_raw_ptr& rhs) noexcept return lhs.ptr == rhs.ptr;
friend bool operator<(const device_raw_ptr& lhs, const device_raw_ptr& rhs) noexcept return lhs.ptr < rhs.ptr;
protected:
T *ptr;
;
template <class T, class U, class V>
std::basic_ostream<U, V>& operator<<(std::basic_ostream<U, V>& os, const device_raw_ptr<T>& other)
os << other.get() << " (device)";
return os;
I am also looking for suggestions on how to order different things inside a class.
Might it be better to initialize with nullptr
instead of the default constructor. This breaks compatibility but I think it might be worth considering the compiler would mostly optimize the assignment if it's immediately overwritten.
c++ c++11 pointers
c++ c++11 pointers
New contributor
New contributor
edited Apr 7 at 17:33
esote
3,03111241
3,03111241
New contributor
asked Apr 7 at 16:15
YashasYashas
1213
1213
New contributor
New contributor
$begingroup$
I just noticed that I am passing rvalues toswap
which accepts non-const lvalue as arguments. You can find it in the copy & move constructor.
$endgroup$
– Yashas
Apr 7 at 16:56
$begingroup$
If anyone did not notice, I messed up therelational_operator
inheritance.
$endgroup$
– Yashas
Apr 8 at 5:14
add a comment |
$begingroup$
I just noticed that I am passing rvalues toswap
which accepts non-const lvalue as arguments. You can find it in the copy & move constructor.
$endgroup$
– Yashas
Apr 7 at 16:56
$begingroup$
If anyone did not notice, I messed up therelational_operator
inheritance.
$endgroup$
– Yashas
Apr 8 at 5:14
$begingroup$
I just noticed that I am passing rvalues to
swap
which accepts non-const lvalue as arguments. You can find it in the copy & move constructor.$endgroup$
– Yashas
Apr 7 at 16:56
$begingroup$
I just noticed that I am passing rvalues to
swap
which accepts non-const lvalue as arguments. You can find it in the copy & move constructor.$endgroup$
– Yashas
Apr 7 at 16:56
$begingroup$
If anyone did not notice, I messed up the
relational_operator
inheritance.$endgroup$
– Yashas
Apr 8 at 5:14
$begingroup$
If anyone did not notice, I messed up the
relational_operator
inheritance.$endgroup$
– Yashas
Apr 8 at 5:14
add a comment |
2 Answers
2
active
oldest
votes
$begingroup$
device_raw_ptr
is extremely cheap to copy, so remove all hints of move-semantics and use ofswap()
.Now you can remove the copy-constructor and copy-assignment, as there is no need to explicitly declare them. Especially making them user-defined must be avoided to keep them trivial.
Kudos on trying to use the approved two-step for
swap()
. Though you get a failing grade anyway because you bungled it by using a qualified call in the second part. Hopefully, C++20 will abolish that nonsense by introducing customization point objects.Yes, you should pass your
device_raw_ptr
by value if you have the choice, as it is a tiny trivial type. Still, refrain from returning a reference to such a temporary.There is no reason
operator-(device_raw_ptr, std::ptrdiff_t)
should not beconstexpr
. Aside from your implementation for some reason delegating tooperator-=
, which is not. Same foroperator+
which usesoperator+=
.Is there any reason you don't support subtracting a
device_raw_ptr
from another?I'm really puzzled why you make the only data-member
protected
instead ofprivate
.
$endgroup$
$begingroup$
Wouldn't the implicitly defined copy-assignment operator return a reference?
$endgroup$
– Yashas
Apr 8 at 5:05
$begingroup$
I have incorporated the changes you suggested. Please check latest code
$endgroup$
– Yashas
Apr 8 at 7:43
add a comment |
$begingroup$
Helper classes
There is no need to use static_cast<bool>
for your comparisons. The relational operators are already bool
values. (If they are not, that is a problem with the definition of the operator for the type T
.)
The standard <utility>
header provides definitions for operator!=
(from operator==
) and operator>
, operator<=
, and operator>=
(from operator<
). There is no need for you to define those four operators if you have the other two (equality and less-than).
Why do you have the relational_operators
struct at all? It shouldn't be necessary.
Implementation
The default constructor for device_raw_ptr
leaves the ptr
member uninitialized. Typically a class like this would initialize ptr
to nullptr
, and you wouldn't need the constructor that takes a std::nullptr_t
object.
The copy assignment operator should just be ptr = other.ptr
, since that is the only thing in your class. The way you have it is nonstandard behavior. You construct a temporary, then pass it as a non-const reference to swap
. This is not supported as part of the language, although some compilers (MSVC) support it as an extension. You're constructing a temporary, doing a swap, then destroying the temporary (a noop in this case). Similarly, the move assignment operator can be simplified to not use the temporary (ptr = other.ptr; other.reset();
, or use three statements with an assignment to a local to avoid problems if you move assign an object to itself).
operator bool
does not need a static_cast
. Perhaps an explicit ptr != nullptr
check, although a pointer will implicitly convert to a bool
.
$endgroup$
$begingroup$
I thought usingstd::rel_ops
is generally frowned upon. stackoverflow.com/questions/6225375/idiomatic-use-of-stdrel-ops suggests usingboost:operators
. I wrote my own version instead.
$endgroup$
– Yashas
Apr 7 at 17:07
$begingroup$
The default constructor leavesptr
unitialized because that's how raw pointers behave (initialized with garbage?). But I am considering the option of initializing it tonullptr
. Thestd::nullptr_t
is a consequence of the aforementioned statement. I intended to have aconstexpr
constructor which setsptr
tonullptr
.
$endgroup$
– Yashas
Apr 7 at 17:09
$begingroup$
For the abuse of swap, I made started (codereview.stackexchange.com/questions/217019/…) using temporaries right after I posted the question. It was an attempt to reuse the constructors to perform move/copy but I think it was overkill for such a simple class.
$endgroup$
– Yashas
Apr 7 at 17:12
$begingroup$
@Yashasstd::rel_ops
is there to avoid having duplicate definitions of those relational operators. If a class does not want some of them (as mentioned in your linked question), then it should define all of them, and= delete
the ones it doesn't want to support. But its up to you to decide how to implement them.
$endgroup$
– 1201ProgramAlarm
Apr 7 at 17:14
$begingroup$
@Yashas leaving raw pointer uninitialized is, generally, a bad idea. The cost of assigning anullptr
someplace where it isn't strictly necessary is outweighed by the predictability and consistent (mis)behavior the NULL value will give you.
$endgroup$
– 1201ProgramAlarm
Apr 7 at 17:16
add a comment |
Your Answer
StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");
StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);
else
createEditor();
);
function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader:
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
,
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);
);
Yashas is a new contributor. Be nice, and check out our Code of Conduct.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f217019%2fsimple-device-fancy-pointer-implementation%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
device_raw_ptr
is extremely cheap to copy, so remove all hints of move-semantics and use ofswap()
.Now you can remove the copy-constructor and copy-assignment, as there is no need to explicitly declare them. Especially making them user-defined must be avoided to keep them trivial.
Kudos on trying to use the approved two-step for
swap()
. Though you get a failing grade anyway because you bungled it by using a qualified call in the second part. Hopefully, C++20 will abolish that nonsense by introducing customization point objects.Yes, you should pass your
device_raw_ptr
by value if you have the choice, as it is a tiny trivial type. Still, refrain from returning a reference to such a temporary.There is no reason
operator-(device_raw_ptr, std::ptrdiff_t)
should not beconstexpr
. Aside from your implementation for some reason delegating tooperator-=
, which is not. Same foroperator+
which usesoperator+=
.Is there any reason you don't support subtracting a
device_raw_ptr
from another?I'm really puzzled why you make the only data-member
protected
instead ofprivate
.
$endgroup$
$begingroup$
Wouldn't the implicitly defined copy-assignment operator return a reference?
$endgroup$
– Yashas
Apr 8 at 5:05
$begingroup$
I have incorporated the changes you suggested. Please check latest code
$endgroup$
– Yashas
Apr 8 at 7:43
add a comment |
$begingroup$
device_raw_ptr
is extremely cheap to copy, so remove all hints of move-semantics and use ofswap()
.Now you can remove the copy-constructor and copy-assignment, as there is no need to explicitly declare them. Especially making them user-defined must be avoided to keep them trivial.
Kudos on trying to use the approved two-step for
swap()
. Though you get a failing grade anyway because you bungled it by using a qualified call in the second part. Hopefully, C++20 will abolish that nonsense by introducing customization point objects.Yes, you should pass your
device_raw_ptr
by value if you have the choice, as it is a tiny trivial type. Still, refrain from returning a reference to such a temporary.There is no reason
operator-(device_raw_ptr, std::ptrdiff_t)
should not beconstexpr
. Aside from your implementation for some reason delegating tooperator-=
, which is not. Same foroperator+
which usesoperator+=
.Is there any reason you don't support subtracting a
device_raw_ptr
from another?I'm really puzzled why you make the only data-member
protected
instead ofprivate
.
$endgroup$
$begingroup$
Wouldn't the implicitly defined copy-assignment operator return a reference?
$endgroup$
– Yashas
Apr 8 at 5:05
$begingroup$
I have incorporated the changes you suggested. Please check latest code
$endgroup$
– Yashas
Apr 8 at 7:43
add a comment |
$begingroup$
device_raw_ptr
is extremely cheap to copy, so remove all hints of move-semantics and use ofswap()
.Now you can remove the copy-constructor and copy-assignment, as there is no need to explicitly declare them. Especially making them user-defined must be avoided to keep them trivial.
Kudos on trying to use the approved two-step for
swap()
. Though you get a failing grade anyway because you bungled it by using a qualified call in the second part. Hopefully, C++20 will abolish that nonsense by introducing customization point objects.Yes, you should pass your
device_raw_ptr
by value if you have the choice, as it is a tiny trivial type. Still, refrain from returning a reference to such a temporary.There is no reason
operator-(device_raw_ptr, std::ptrdiff_t)
should not beconstexpr
. Aside from your implementation for some reason delegating tooperator-=
, which is not. Same foroperator+
which usesoperator+=
.Is there any reason you don't support subtracting a
device_raw_ptr
from another?I'm really puzzled why you make the only data-member
protected
instead ofprivate
.
$endgroup$
device_raw_ptr
is extremely cheap to copy, so remove all hints of move-semantics and use ofswap()
.Now you can remove the copy-constructor and copy-assignment, as there is no need to explicitly declare them. Especially making them user-defined must be avoided to keep them trivial.
Kudos on trying to use the approved two-step for
swap()
. Though you get a failing grade anyway because you bungled it by using a qualified call in the second part. Hopefully, C++20 will abolish that nonsense by introducing customization point objects.Yes, you should pass your
device_raw_ptr
by value if you have the choice, as it is a tiny trivial type. Still, refrain from returning a reference to such a temporary.There is no reason
operator-(device_raw_ptr, std::ptrdiff_t)
should not beconstexpr
. Aside from your implementation for some reason delegating tooperator-=
, which is not. Same foroperator+
which usesoperator+=
.Is there any reason you don't support subtracting a
device_raw_ptr
from another?I'm really puzzled why you make the only data-member
protected
instead ofprivate
.
answered Apr 7 at 20:16
DeduplicatorDeduplicator
11.9k1950
11.9k1950
$begingroup$
Wouldn't the implicitly defined copy-assignment operator return a reference?
$endgroup$
– Yashas
Apr 8 at 5:05
$begingroup$
I have incorporated the changes you suggested. Please check latest code
$endgroup$
– Yashas
Apr 8 at 7:43
add a comment |
$begingroup$
Wouldn't the implicitly defined copy-assignment operator return a reference?
$endgroup$
– Yashas
Apr 8 at 5:05
$begingroup$
I have incorporated the changes you suggested. Please check latest code
$endgroup$
– Yashas
Apr 8 at 7:43
$begingroup$
Wouldn't the implicitly defined copy-assignment operator return a reference?
$endgroup$
– Yashas
Apr 8 at 5:05
$begingroup$
Wouldn't the implicitly defined copy-assignment operator return a reference?
$endgroup$
– Yashas
Apr 8 at 5:05
$begingroup$
I have incorporated the changes you suggested. Please check latest code
$endgroup$
– Yashas
Apr 8 at 7:43
$begingroup$
I have incorporated the changes you suggested. Please check latest code
$endgroup$
– Yashas
Apr 8 at 7:43
add a comment |
$begingroup$
Helper classes
There is no need to use static_cast<bool>
for your comparisons. The relational operators are already bool
values. (If they are not, that is a problem with the definition of the operator for the type T
.)
The standard <utility>
header provides definitions for operator!=
(from operator==
) and operator>
, operator<=
, and operator>=
(from operator<
). There is no need for you to define those four operators if you have the other two (equality and less-than).
Why do you have the relational_operators
struct at all? It shouldn't be necessary.
Implementation
The default constructor for device_raw_ptr
leaves the ptr
member uninitialized. Typically a class like this would initialize ptr
to nullptr
, and you wouldn't need the constructor that takes a std::nullptr_t
object.
The copy assignment operator should just be ptr = other.ptr
, since that is the only thing in your class. The way you have it is nonstandard behavior. You construct a temporary, then pass it as a non-const reference to swap
. This is not supported as part of the language, although some compilers (MSVC) support it as an extension. You're constructing a temporary, doing a swap, then destroying the temporary (a noop in this case). Similarly, the move assignment operator can be simplified to not use the temporary (ptr = other.ptr; other.reset();
, or use three statements with an assignment to a local to avoid problems if you move assign an object to itself).
operator bool
does not need a static_cast
. Perhaps an explicit ptr != nullptr
check, although a pointer will implicitly convert to a bool
.
$endgroup$
$begingroup$
I thought usingstd::rel_ops
is generally frowned upon. stackoverflow.com/questions/6225375/idiomatic-use-of-stdrel-ops suggests usingboost:operators
. I wrote my own version instead.
$endgroup$
– Yashas
Apr 7 at 17:07
$begingroup$
The default constructor leavesptr
unitialized because that's how raw pointers behave (initialized with garbage?). But I am considering the option of initializing it tonullptr
. Thestd::nullptr_t
is a consequence of the aforementioned statement. I intended to have aconstexpr
constructor which setsptr
tonullptr
.
$endgroup$
– Yashas
Apr 7 at 17:09
$begingroup$
For the abuse of swap, I made started (codereview.stackexchange.com/questions/217019/…) using temporaries right after I posted the question. It was an attempt to reuse the constructors to perform move/copy but I think it was overkill for such a simple class.
$endgroup$
– Yashas
Apr 7 at 17:12
$begingroup$
@Yashasstd::rel_ops
is there to avoid having duplicate definitions of those relational operators. If a class does not want some of them (as mentioned in your linked question), then it should define all of them, and= delete
the ones it doesn't want to support. But its up to you to decide how to implement them.
$endgroup$
– 1201ProgramAlarm
Apr 7 at 17:14
$begingroup$
@Yashas leaving raw pointer uninitialized is, generally, a bad idea. The cost of assigning anullptr
someplace where it isn't strictly necessary is outweighed by the predictability and consistent (mis)behavior the NULL value will give you.
$endgroup$
– 1201ProgramAlarm
Apr 7 at 17:16
add a comment |
$begingroup$
Helper classes
There is no need to use static_cast<bool>
for your comparisons. The relational operators are already bool
values. (If they are not, that is a problem with the definition of the operator for the type T
.)
The standard <utility>
header provides definitions for operator!=
(from operator==
) and operator>
, operator<=
, and operator>=
(from operator<
). There is no need for you to define those four operators if you have the other two (equality and less-than).
Why do you have the relational_operators
struct at all? It shouldn't be necessary.
Implementation
The default constructor for device_raw_ptr
leaves the ptr
member uninitialized. Typically a class like this would initialize ptr
to nullptr
, and you wouldn't need the constructor that takes a std::nullptr_t
object.
The copy assignment operator should just be ptr = other.ptr
, since that is the only thing in your class. The way you have it is nonstandard behavior. You construct a temporary, then pass it as a non-const reference to swap
. This is not supported as part of the language, although some compilers (MSVC) support it as an extension. You're constructing a temporary, doing a swap, then destroying the temporary (a noop in this case). Similarly, the move assignment operator can be simplified to not use the temporary (ptr = other.ptr; other.reset();
, or use three statements with an assignment to a local to avoid problems if you move assign an object to itself).
operator bool
does not need a static_cast
. Perhaps an explicit ptr != nullptr
check, although a pointer will implicitly convert to a bool
.
$endgroup$
$begingroup$
I thought usingstd::rel_ops
is generally frowned upon. stackoverflow.com/questions/6225375/idiomatic-use-of-stdrel-ops suggests usingboost:operators
. I wrote my own version instead.
$endgroup$
– Yashas
Apr 7 at 17:07
$begingroup$
The default constructor leavesptr
unitialized because that's how raw pointers behave (initialized with garbage?). But I am considering the option of initializing it tonullptr
. Thestd::nullptr_t
is a consequence of the aforementioned statement. I intended to have aconstexpr
constructor which setsptr
tonullptr
.
$endgroup$
– Yashas
Apr 7 at 17:09
$begingroup$
For the abuse of swap, I made started (codereview.stackexchange.com/questions/217019/…) using temporaries right after I posted the question. It was an attempt to reuse the constructors to perform move/copy but I think it was overkill for such a simple class.
$endgroup$
– Yashas
Apr 7 at 17:12
$begingroup$
@Yashasstd::rel_ops
is there to avoid having duplicate definitions of those relational operators. If a class does not want some of them (as mentioned in your linked question), then it should define all of them, and= delete
the ones it doesn't want to support. But its up to you to decide how to implement them.
$endgroup$
– 1201ProgramAlarm
Apr 7 at 17:14
$begingroup$
@Yashas leaving raw pointer uninitialized is, generally, a bad idea. The cost of assigning anullptr
someplace where it isn't strictly necessary is outweighed by the predictability and consistent (mis)behavior the NULL value will give you.
$endgroup$
– 1201ProgramAlarm
Apr 7 at 17:16
add a comment |
$begingroup$
Helper classes
There is no need to use static_cast<bool>
for your comparisons. The relational operators are already bool
values. (If they are not, that is a problem with the definition of the operator for the type T
.)
The standard <utility>
header provides definitions for operator!=
(from operator==
) and operator>
, operator<=
, and operator>=
(from operator<
). There is no need for you to define those four operators if you have the other two (equality and less-than).
Why do you have the relational_operators
struct at all? It shouldn't be necessary.
Implementation
The default constructor for device_raw_ptr
leaves the ptr
member uninitialized. Typically a class like this would initialize ptr
to nullptr
, and you wouldn't need the constructor that takes a std::nullptr_t
object.
The copy assignment operator should just be ptr = other.ptr
, since that is the only thing in your class. The way you have it is nonstandard behavior. You construct a temporary, then pass it as a non-const reference to swap
. This is not supported as part of the language, although some compilers (MSVC) support it as an extension. You're constructing a temporary, doing a swap, then destroying the temporary (a noop in this case). Similarly, the move assignment operator can be simplified to not use the temporary (ptr = other.ptr; other.reset();
, or use three statements with an assignment to a local to avoid problems if you move assign an object to itself).
operator bool
does not need a static_cast
. Perhaps an explicit ptr != nullptr
check, although a pointer will implicitly convert to a bool
.
$endgroup$
Helper classes
There is no need to use static_cast<bool>
for your comparisons. The relational operators are already bool
values. (If they are not, that is a problem with the definition of the operator for the type T
.)
The standard <utility>
header provides definitions for operator!=
(from operator==
) and operator>
, operator<=
, and operator>=
(from operator<
). There is no need for you to define those four operators if you have the other two (equality and less-than).
Why do you have the relational_operators
struct at all? It shouldn't be necessary.
Implementation
The default constructor for device_raw_ptr
leaves the ptr
member uninitialized. Typically a class like this would initialize ptr
to nullptr
, and you wouldn't need the constructor that takes a std::nullptr_t
object.
The copy assignment operator should just be ptr = other.ptr
, since that is the only thing in your class. The way you have it is nonstandard behavior. You construct a temporary, then pass it as a non-const reference to swap
. This is not supported as part of the language, although some compilers (MSVC) support it as an extension. You're constructing a temporary, doing a swap, then destroying the temporary (a noop in this case). Similarly, the move assignment operator can be simplified to not use the temporary (ptr = other.ptr; other.reset();
, or use three statements with an assignment to a local to avoid problems if you move assign an object to itself).
operator bool
does not need a static_cast
. Perhaps an explicit ptr != nullptr
check, although a pointer will implicitly convert to a bool
.
answered Apr 7 at 17:00
1201ProgramAlarm1201ProgramAlarm
3,7232925
3,7232925
$begingroup$
I thought usingstd::rel_ops
is generally frowned upon. stackoverflow.com/questions/6225375/idiomatic-use-of-stdrel-ops suggests usingboost:operators
. I wrote my own version instead.
$endgroup$
– Yashas
Apr 7 at 17:07
$begingroup$
The default constructor leavesptr
unitialized because that's how raw pointers behave (initialized with garbage?). But I am considering the option of initializing it tonullptr
. Thestd::nullptr_t
is a consequence of the aforementioned statement. I intended to have aconstexpr
constructor which setsptr
tonullptr
.
$endgroup$
– Yashas
Apr 7 at 17:09
$begingroup$
For the abuse of swap, I made started (codereview.stackexchange.com/questions/217019/…) using temporaries right after I posted the question. It was an attempt to reuse the constructors to perform move/copy but I think it was overkill for such a simple class.
$endgroup$
– Yashas
Apr 7 at 17:12
$begingroup$
@Yashasstd::rel_ops
is there to avoid having duplicate definitions of those relational operators. If a class does not want some of them (as mentioned in your linked question), then it should define all of them, and= delete
the ones it doesn't want to support. But its up to you to decide how to implement them.
$endgroup$
– 1201ProgramAlarm
Apr 7 at 17:14
$begingroup$
@Yashas leaving raw pointer uninitialized is, generally, a bad idea. The cost of assigning anullptr
someplace where it isn't strictly necessary is outweighed by the predictability and consistent (mis)behavior the NULL value will give you.
$endgroup$
– 1201ProgramAlarm
Apr 7 at 17:16
add a comment |
$begingroup$
I thought usingstd::rel_ops
is generally frowned upon. stackoverflow.com/questions/6225375/idiomatic-use-of-stdrel-ops suggests usingboost:operators
. I wrote my own version instead.
$endgroup$
– Yashas
Apr 7 at 17:07
$begingroup$
The default constructor leavesptr
unitialized because that's how raw pointers behave (initialized with garbage?). But I am considering the option of initializing it tonullptr
. Thestd::nullptr_t
is a consequence of the aforementioned statement. I intended to have aconstexpr
constructor which setsptr
tonullptr
.
$endgroup$
– Yashas
Apr 7 at 17:09
$begingroup$
For the abuse of swap, I made started (codereview.stackexchange.com/questions/217019/…) using temporaries right after I posted the question. It was an attempt to reuse the constructors to perform move/copy but I think it was overkill for such a simple class.
$endgroup$
– Yashas
Apr 7 at 17:12
$begingroup$
@Yashasstd::rel_ops
is there to avoid having duplicate definitions of those relational operators. If a class does not want some of them (as mentioned in your linked question), then it should define all of them, and= delete
the ones it doesn't want to support. But its up to you to decide how to implement them.
$endgroup$
– 1201ProgramAlarm
Apr 7 at 17:14
$begingroup$
@Yashas leaving raw pointer uninitialized is, generally, a bad idea. The cost of assigning anullptr
someplace where it isn't strictly necessary is outweighed by the predictability and consistent (mis)behavior the NULL value will give you.
$endgroup$
– 1201ProgramAlarm
Apr 7 at 17:16
$begingroup$
I thought using
std::rel_ops
is generally frowned upon. stackoverflow.com/questions/6225375/idiomatic-use-of-stdrel-ops suggests using boost:operators
. I wrote my own version instead.$endgroup$
– Yashas
Apr 7 at 17:07
$begingroup$
I thought using
std::rel_ops
is generally frowned upon. stackoverflow.com/questions/6225375/idiomatic-use-of-stdrel-ops suggests using boost:operators
. I wrote my own version instead.$endgroup$
– Yashas
Apr 7 at 17:07
$begingroup$
The default constructor leaves
ptr
unitialized because that's how raw pointers behave (initialized with garbage?). But I am considering the option of initializing it to nullptr
. The std::nullptr_t
is a consequence of the aforementioned statement. I intended to have a constexpr
constructor which sets ptr
to nullptr
.$endgroup$
– Yashas
Apr 7 at 17:09
$begingroup$
The default constructor leaves
ptr
unitialized because that's how raw pointers behave (initialized with garbage?). But I am considering the option of initializing it to nullptr
. The std::nullptr_t
is a consequence of the aforementioned statement. I intended to have a constexpr
constructor which sets ptr
to nullptr
.$endgroup$
– Yashas
Apr 7 at 17:09
$begingroup$
For the abuse of swap, I made started (codereview.stackexchange.com/questions/217019/…) using temporaries right after I posted the question. It was an attempt to reuse the constructors to perform move/copy but I think it was overkill for such a simple class.
$endgroup$
– Yashas
Apr 7 at 17:12
$begingroup$
For the abuse of swap, I made started (codereview.stackexchange.com/questions/217019/…) using temporaries right after I posted the question. It was an attempt to reuse the constructors to perform move/copy but I think it was overkill for such a simple class.
$endgroup$
– Yashas
Apr 7 at 17:12
$begingroup$
@Yashas
std::rel_ops
is there to avoid having duplicate definitions of those relational operators. If a class does not want some of them (as mentioned in your linked question), then it should define all of them, and = delete
the ones it doesn't want to support. But its up to you to decide how to implement them.$endgroup$
– 1201ProgramAlarm
Apr 7 at 17:14
$begingroup$
@Yashas
std::rel_ops
is there to avoid having duplicate definitions of those relational operators. If a class does not want some of them (as mentioned in your linked question), then it should define all of them, and = delete
the ones it doesn't want to support. But its up to you to decide how to implement them.$endgroup$
– 1201ProgramAlarm
Apr 7 at 17:14
$begingroup$
@Yashas leaving raw pointer uninitialized is, generally, a bad idea. The cost of assigning a
nullptr
someplace where it isn't strictly necessary is outweighed by the predictability and consistent (mis)behavior the NULL value will give you.$endgroup$
– 1201ProgramAlarm
Apr 7 at 17:16
$begingroup$
@Yashas leaving raw pointer uninitialized is, generally, a bad idea. The cost of assigning a
nullptr
someplace where it isn't strictly necessary is outweighed by the predictability and consistent (mis)behavior the NULL value will give you.$endgroup$
– 1201ProgramAlarm
Apr 7 at 17:16
add a comment |
Yashas is a new contributor. Be nice, and check out our Code of Conduct.
Yashas is a new contributor. Be nice, and check out our Code of Conduct.
Yashas is a new contributor. Be nice, and check out our Code of Conduct.
Yashas is a new contributor. Be nice, and check out our Code of Conduct.
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f217019%2fsimple-device-fancy-pointer-implementation%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
$begingroup$
I just noticed that I am passing rvalues to
swap
which accepts non-const lvalue as arguments. You can find it in the copy & move constructor.$endgroup$
– Yashas
Apr 7 at 16:56
$begingroup$
If anyone did not notice, I messed up the
relational_operator
inheritance.$endgroup$
– Yashas
Apr 8 at 5:14