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.

Sign up for GitHub

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

Jump to bottom

Generalize LLPointer's comparison operators to allow comparable LLPointer types. #2681

Merged
merged 4 commits into from
Sep 25, 2024
Merged

Generalize LLPointer's comparison operators to allow comparable LLPointer types. #2681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
705fd68
Make `LLPointer` equality comparisons accept not-identical types.
nat-goodspeed Sep 24, 2024
1a682de
LLConstPointer<T> is the same as LLPointer<const T>.
nat-goodspeed Sep 24, 2024
98c829a
Use copy-and-swap idiom for LLPointer's assignment operators.
nat-goodspeed Sep 25, 2024
5d7b3ab
Explain why apparently redundant LLPointer methods are necessary.
nat-goodspeed Sep 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
256 changes: 55 additions & 201 deletions indra/llcommon/llpointer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

#include "llerror.h" // *TODO: consider eliminating this
#include "llmutex.h"
#include <utility> // std::swap()

//----------------------------------------------------------------------------
// RefCount objects should generally only be accessed by way of LLPointer<>'s
Expand Down Expand Up @@ -60,6 +61,13 @@ template <class Type> class LLPointer
ref();
}

// Even though the template constructors below accepting
// (const LLPointer<Subclass>&) and (LLPointer<Subclass>&&) appear to
// subsume these specific (const LLPointer<Type>&) and (LLPointer<Type>&&)
// constructors, the compiler recognizes these as The Copy Constructor and
// The Move Constructor, respectively. In other words, even in the
// presence of the LLPointer<Subclass> constructors, we still must specify
// the LLPointer<Type> constructors.
LLPointer(const LLPointer<Type>& ptr) :
mPointer(ptr.mPointer)
{
Expand Down Expand Up @@ -105,61 +113,79 @@ template <class Type> class LLPointer
bool notNull() const { return (mPointer != nullptr); }

operator Type*() const { return mPointer; }
bool operator !=(Type* ptr) const { return (mPointer != ptr); }
bool operator ==(Type* ptr) const { return (mPointer == ptr); }
bool operator ==(const LLPointer<Type>& ptr) const { return (mPointer == ptr.mPointer); }
bool operator < (const LLPointer<Type>& ptr) const { return (mPointer < ptr.mPointer); }
bool operator > (const LLPointer<Type>& ptr) const { return (mPointer > ptr.mPointer); }
template <typename Type1>
bool operator !=(Type1* ptr) const { return (mPointer != ptr); }
template <typename Type1>
bool operator ==(Type1* ptr) const { return (mPointer == ptr); }
template <typename Type1>
bool operator !=(const LLPointer<Type1>& ptr) const { return (mPointer != ptr.mPointer); }
template <typename Type1>
bool operator ==(const LLPointer<Type1>& ptr) const { return (mPointer == ptr.mPointer); }
bool operator < (const LLPointer<Type>& ptr) const { return (mPointer < ptr.mPointer); }
bool operator > (const LLPointer<Type>& ptr) const { return (mPointer > ptr.mPointer); }

LLPointer<Type>& operator =(Type* ptr)
{
assign(ptr);
// copy-and-swap idiom, see http://gotw.ca/gotw/059.htm
LLPointer temp(ptr);
using std::swap; // per Swappable convention
swap(*this, temp);
return *this;
}

// Even though the template assignment operators below accepting
// (const LLPointer<Subclass>&) and (LLPointer<Subclass>&&) appear to
// subsume these specific (const LLPointer<Type>&) and (LLPointer<Type>&&)
// assignment operators, the compiler recognizes these as Copy Assignment
// and Move Assignment, respectively. In other words, even in the presence
// of the LLPointer<Subclass> assignment operators, we still must specify
// the LLPointer<Type> operators.
LLPointer<Type>& operator =(const LLPointer<Type>& ptr)
{
assign(ptr);
LLPointer temp(ptr);
using std::swap; // per Swappable convention
swap(*this, temp);
return *this;
}

LLPointer<Type>& operator =(LLPointer<Type>&& ptr)
{
if (mPointer != ptr.mPointer)
{
unref();
mPointer = ptr.mPointer;
ptr.mPointer = nullptr;
}
LLPointer temp(std::move(ptr));
using std::swap; // per Swappable convention
swap(*this, temp);
return *this;
}

// support assignment up the type hierarchy. See Item 45 in Effective C++, 3rd Ed.
template<typename Subclass>
LLPointer<Type>& operator =(const LLPointer<Subclass>& ptr)
{
assign(ptr.get());
LLPointer temp(ptr);
using std::swap; // per Swappable convention
swap(*this, temp);
return *this;
}

template<typename Subclass>
LLPointer<Type>& operator =(LLPointer<Subclass>&& ptr)
{
if (mPointer != ptr.mPointer)
{
unref();
mPointer = ptr.mPointer;
ptr.mPointer = nullptr;
}
LLPointer temp(std::move(ptr));
using std::swap; // per Swappable convention
swap(*this, temp);
return *this;
}

// Just exchange the pointers, which will not change the reference counts.
static void swap(LLPointer<Type>& a, LLPointer<Type>& b)
{
Type* temp = a.mPointer;
a.mPointer = b.mPointer;
b.mPointer = temp;
using std::swap; // per Swappable convention
swap(a.mPointer, b.mPointer);
}

// Put swap() overload in the global namespace, per Swappable convention
friend void swap(LLPointer<Type>& a, LLPointer<Type>& b)
{
LLPointer<Type>::swap(a, b);
}

protected:
Expand Down Expand Up @@ -191,184 +217,12 @@ template <class Type> class LLPointer
}
#endif // LL_LIBRARY_INCLUDE

void assign(const LLPointer<Type>& ptr)
{
if (mPointer != ptr.mPointer)
{
unref();
mPointer = ptr.mPointer;
ref();
}
}

protected:
Type* mPointer;
};

template <class Type> class LLConstPointer
{
template<typename Subclass>
friend class LLConstPointer;
public:
LLConstPointer() :
mPointer(nullptr)
{
}

LLConstPointer(const Type* ptr) :
mPointer(ptr)
{
ref();
}

LLConstPointer(const LLConstPointer<Type>& ptr) :
mPointer(ptr.mPointer)
{
ref();
}

LLConstPointer(LLConstPointer<Type>&& ptr) noexcept
{
mPointer = ptr.mPointer;
ptr.mPointer = nullptr;
}

// support conversion up the type hierarchy. See Item 45 in Effective C++, 3rd Ed.
template<typename Subclass>
LLConstPointer(const LLConstPointer<Subclass>& ptr) :
mPointer(ptr.get())
{
ref();
}

template<typename Subclass>
LLConstPointer(LLConstPointer<Subclass>&& ptr) noexcept :
mPointer(ptr.get())
{
ptr.mPointer = nullptr;
}

~LLConstPointer()
{
unref();
}

const Type* get() const { return mPointer; }
const Type* operator->() const { return mPointer; }
const Type& operator*() const { return *mPointer; }

operator BOOL() const { return (mPointer != nullptr); }
operator bool() const { return (mPointer != nullptr); }
bool operator!() const { return (mPointer == nullptr); }
bool isNull() const { return (mPointer == nullptr); }
bool notNull() const { return (mPointer != nullptr); }

operator const Type*() const { return mPointer; }
bool operator !=(const Type* ptr) const { return (mPointer != ptr); }
bool operator ==(const Type* ptr) const { return (mPointer == ptr); }
bool operator ==(const LLConstPointer<Type>& ptr) const { return (mPointer == ptr.mPointer); }
bool operator < (const LLConstPointer<Type>& ptr) const { return (mPointer < ptr.mPointer); }
bool operator > (const LLConstPointer<Type>& ptr) const { return (mPointer > ptr.mPointer); }

LLConstPointer<Type>& operator =(const Type* ptr)
{
if( mPointer != ptr )
{
unref();
mPointer = ptr;
ref();
}

return *this;
}

LLConstPointer<Type>& operator =(const LLConstPointer<Type>& ptr)
{
if( mPointer != ptr.mPointer )
{
unref();
mPointer = ptr.mPointer;
ref();
}
return *this;
}

LLConstPointer<Type>& operator =(LLConstPointer<Type>&& ptr)
{
if (mPointer != ptr.mPointer)
{
unref();
mPointer = ptr.mPointer;
ptr.mPointer = nullptr;
}
return *this;
}

// support assignment up the type hierarchy. See Item 45 in Effective C++, 3rd Ed.
template<typename Subclass>
LLConstPointer<Type>& operator =(const LLConstPointer<Subclass>& ptr)
{
if( mPointer != ptr.get() )
{
unref();
mPointer = ptr.get();
ref();
}
return *this;
}

template<typename Subclass>
LLConstPointer<Type>& operator =(LLConstPointer<Subclass>&& ptr)
{
if (mPointer != ptr.mPointer)
{
unref();
mPointer = ptr.mPointer;
ptr.mPointer = nullptr;
}
return *this;
}

// Just exchange the pointers, which will not change the reference counts.
static void swap(LLConstPointer<Type>& a, LLConstPointer<Type>& b)
{
const Type* temp = a.mPointer;
a.mPointer = b.mPointer;
b.mPointer = temp;
}

protected:
#ifdef LL_LIBRARY_INCLUDE
void ref();
void unref();
#else // LL_LIBRARY_INCLUDE
void ref()
{
if (mPointer)
{
mPointer->ref();
}
}

void unref()
{
if (mPointer)
{
const Type *temp = mPointer;
mPointer = nullptr;
temp->unref();
if (mPointer != nullptr)
{
LL_WARNS() << "Unreference did assignment to non-NULL because of destructor" << LL_ENDL;
unref();
}
}
}
#endif // LL_LIBRARY_INCLUDE

protected:
const Type* mPointer;
};
template <typename Type>
using LLConstPointer = LLPointer<const Type>;

template<typename Type>
class LLCopyOnWritePointer : public LLPointer<Type>
Expand Down Expand Up @@ -418,14 +272,14 @@ class LLCopyOnWritePointer : public LLPointer<Type>
bool mStayUnique;
};

template<typename Type>
bool operator!=(Type* lhs, const LLPointer<Type>& rhs)
template<typename Type0, typename Type1>
bool operator!=(Type0* lhs, const LLPointer<Type1>& rhs)
{
return (lhs != rhs.get());
}

template<typename Type>
bool operator==(Type* lhs, const LLPointer<Type>& rhs)
template<typename Type0, typename Type1>
bool operator==(Type0* lhs, const LLPointer<Type1>& rhs)
{
return (lhs == rhs.get());
}
Expand Down
2 changes: 1 addition & 1 deletion indra/newview/llviewerwindow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1331,7 +1331,7 @@ LLWindowCallbacks::DragNDropResult LLViewerWindow::handleDragNDrop( LLWindow *wi
// Check the whitelist, if there's media (otherwise just show it)
if (te->getMediaData() == NULL || te->getMediaData()->checkCandidateUrl(url))
{
if (obj != mDragHoveredObject.get())
if (obj != mDragHoveredObject)
{
// Highlight the dragged object
LLSelectMgr::getInstance()->unhighlightObjectOnly(mDragHoveredObject);
Expand Down
Loading