C++: Add shared_ptr and unique_ptr implementations#5541
Conversation
MathiasVP
force-pushed
the
definitions-for-unique_ptr
branch
from
09659ab to
8dc7b64
Compare
| namespace detail { | ||
| template<typename T> | ||
| class compressed_pair_element { | ||
| T element; |
There was a problem hiding this comment.
Where does the code in this file come from? (copied from a standard, reverse engineered from QL, written ad-hoc...)
| template<class T, class Deleter = default_delete<T> > | ||
| class unique_ptr { | ||
| private: | ||
| detail::compressed_pair<T*, Deleter> data; |
There was a problem hiding this comment.
I notice that there's a little bit more implementation in these classes than we've used before (in STL tests), presumably so that we can see what flows are computed without the help of models?
There was a problem hiding this comment.
Exactly. Hopefully, the new definitions don't interfere with the existing declarations (that previously didn't have any definitions).
| shared_ptr(const shared_ptr& s) noexcept : ptr(s.ptr), ctrl(s.ctrl) { | ||
| inc(); | ||
| } | ||
| shared_ptr(shared_ptr&& s) noexcept = default; |
There was a problem hiding this comment.
In the standard (the version I'm reading anyway) these are template functions, it appears the type of the parameter doesn't need to exactly match the type of the pointer that's being constructed.
There was a problem hiding this comment.
True. A standards-conforming implementation will also need a templated copy constructor like template<class U> shared_ptr( const shared_ptr<U>&). I'm guessing this is to allow implicit conversions from T to U when that makes sense. Defining what "when that makes sense" means requires a bunch of ugly templated structs, though. So I decided to not do that.
There was a problem hiding this comment.
Fair enough. It will be worth testing the models we produce on real world code to ensure that none of these differences turn out to matter.
geoffw0
left a comment
geoffw0
left a comment
There was a problem hiding this comment.
LGTM.
I'll merge this tomorrow if nobody else comments.
…ults (and lose the field conflation, yay) It might be due to github#3364.
|
Is this still ready to merge? |
Yep. The changes in 09ba25f are all good. |
2 participants
This is my probably-non-standard-compliant-and-not-well-tested implementation of
std::shared_ptrandstd::unique_ptr. It does the job in that it shows some problems in our dataflow/taint tracking support for smart pointers. Hopefully, we can use this as a basis for some more realistic tests soon.(Part of https://github.com/github/codeql-c-analysis-team/issues/255).