-
Notifications
You must be signed in to change notification settings - Fork 288
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
Unified object provider backend #911
base: main
Are you sure you want to change the base?
Changes from all commits
95f0438
42ec787
0b1d87f
2d04507
7e1e57e
e4ed60e
2e71ca8
a4200ec
4bbc46a
9d999d8
e0ba623
163d9cd
2a54205
0912655
5d7288c
10189ac
2567607
ecdd000
8e7ae61
850d7c9
548be38
6dbbf06
74b86fd
5171d3e
d69ed8f
2df42d5
5f1786e
6f5a7ba
93303c3
8a26c8b
aa7e52f
d46a4d6
b045c64
104758f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,4 +10,5 @@ namespace vcpkg | |
|
||
struct FileSink; | ||
struct CombiningSink; | ||
struct BGMessageSink; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -249,10 +249,22 @@ namespace vcpkg | |
{ | ||
constexpr OptionalStorage() noexcept : m_t(nullptr) { } | ||
constexpr OptionalStorage(const T& t) : m_t(&t) { } | ||
constexpr OptionalStorage(const Optional<T>& t) : m_t(t.get()) { } | ||
constexpr OptionalStorage(const Optional<const T>& t) : m_t(t.get()) { } | ||
constexpr OptionalStorage(Optional<T>&& t) = delete; | ||
constexpr OptionalStorage(Optional<const T>&& t) = delete; | ||
template<typename U, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain why this change was necessary? Optional stores T, not T*, so whether T is abstract should not matter. ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From Discord: the problem is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also don't see how you would use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the question is if the following is a valid expressions: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I see, the problem is that given
I believe the correct fix is just to put the
should be
and in
should just be
|
||
std::enable_if_t<std::is_same_v<T, std::decay_t<U>> && !std::is_abstract_v<T>, int*> = nullptr> | ||
constexpr OptionalStorage(const Optional<U>& t) : m_t(t.get()) | ||
{ | ||
} | ||
template<typename U, | ||
std::enable_if_t<std::is_same_v<T, std::decay_t<U>> && !std::is_abstract_v<T>, int*> = nullptr> | ||
constexpr OptionalStorage(const Optional<const U>& t) : m_t(t.get()) | ||
{ | ||
} | ||
template<typename U, | ||
std::enable_if_t<std::is_same_v<T, std::decay_t<U>> && !std::is_abstract_v<T>, int*> = nullptr> | ||
constexpr OptionalStorage(Optional<U>&& t) = delete; | ||
template<typename U, | ||
std::enable_if_t<std::is_same_v<T, std::decay_t<U>> && !std::is_abstract_v<T>, int*> = nullptr> | ||
constexpr OptionalStorage(Optional<const U>&& t) = delete; | ||
|
||
constexpr bool has_value() const { return m_t != nullptr; } | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think you meant to do that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests do not build currently (expected) but I want to run the e2e tests in the ci, so I have to disable the unit tests temporarily.