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

Implement workaround for LLVM-46269 by ensuring that constrained destructor appears after the unconstrained one #2630

Merged
merged 2 commits into from
Apr 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 3 additions & 4 deletions stl/inc/iterator
Original file line number Diff line number Diff line change
Expand Up @@ -618,15 +618,14 @@ public:
}
}

// clang-format off
constexpr ~_Variantish() requires is_trivially_destructible_v<_It> && is_trivially_destructible_v<_Se> = default;
// clang-format on

constexpr ~_Variantish() {
_Raw_clear();
}

// TRANSITION, LLVM-46269, destructor order is significant
// clang-format off
constexpr ~_Variantish() requires is_trivially_destructible_v<_It> && is_trivially_destructible_v<_Se> = default;

constexpr _Variantish& operator=(const _Variantish&) requires is_trivially_destructible_v<_It>
&& is_trivially_destructible_v<_Se>
&& is_trivially_copy_constructible_v<_It>
Expand Down
15 changes: 7 additions & 8 deletions stl/inc/ranges
Original file line number Diff line number Diff line change
Expand Up @@ -271,17 +271,16 @@ namespace ranges {
is_nothrow_constructible_v<_Ty, _Types...>) // strengthened
: _Val(_STD forward<_Types>(_Args)...), _Engaged{true} {}

// clang-format off
~_Copyable_box() requires is_trivially_destructible_v<_Ty> = default;
// clang-format on

constexpr ~_Copyable_box() {
if (_Engaged) {
_Val.~_Ty();
}
}

// TRANSITION, LLVM-46269, destructor order is significant
// clang-format off
~_Copyable_box() requires is_trivially_destructible_v<_Ty> = default;

_Copyable_box(const _Copyable_box&) requires is_trivially_copy_constructible_v<_Ty> = default;
// clang-format on

Expand Down Expand Up @@ -474,17 +473,16 @@ namespace ranges {
public:
constexpr _Defaultabox() noexcept {}

// clang-format off
~_Defaultabox() requires is_trivially_destructible_v<_Ty> = default;
// clang-format on

constexpr ~_Defaultabox() {
if (_Engaged) {
_Val.~_Ty();
}
}

// TRANSITION, LLVM-46269, destructor order is significant
// clang-format off
~_Defaultabox() requires is_trivially_destructible_v<_Ty> = default;

_Defaultabox(const _Defaultabox&)
requires copy_constructible<_Ty> && is_trivially_copy_constructible_v<_Ty> = default;
// clang-format on
Expand Down Expand Up @@ -693,6 +691,7 @@ namespace ranges {
}
}

// TRANSITION, LLVM-46269, destructor order is significant
// clang-format off
~_Non_propagating_cache() requires is_trivially_destructible_v<_Ty> = default;
// clang-format on
Expand Down
23 changes: 23 additions & 0 deletions tests/std/tests/P0896R4_common_iterator/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,29 @@ constexpr bool test_lwg_3574() {
return true;
}

// Validate that _Variantish works when fed with a non-trivially-destructible type
void test_non_trivially_destructible_type() { // COMPILE-ONLY
struct non_trivially_destructible_input_iterator {
using difference_type = int;
using value_type = int;

~non_trivially_destructible_input_iterator() {}

non_trivially_destructible_input_iterator& operator++() {
return *this;
}
void operator++(int) {}
int operator*() const {
return 0;
}
bool operator==(default_sentinel_t) const {
return true;
}
};

common_iterator<non_trivially_destructible_input_iterator, default_sentinel_t> it;
}

int main() {
with_writable_iterators<instantiator, P>::call();
static_assert(with_writable_iterators<instantiator, P>::call());
Expand Down
32 changes: 32 additions & 0 deletions tests/std/tests/P0896R4_views_join/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,38 @@ struct Immovable {
Immovable& operator=(Immovable&&) = delete;
};

// Validate that the _Defaultabox primary template works when fed with a non-trivially-destructible type
void test_non_trivially_destructible_type() { // COMPILE-ONLY
struct non_trivially_destructible_input_iterator {
using difference_type = int;
using value_type = int;

~non_trivially_destructible_input_iterator() {}

// To test the correct specialization of _Defaultabox, this type must not be default constructible.
non_trivially_destructible_input_iterator() = delete;
Comment on lines +482 to +483
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised that this doesn't trigger compiler warnings about "this class has no constructors" (usually I'd expect to see an (int, int) constructor or something similar to avoid this), but no change requested as there are apparently no test-breaking warnings.


non_trivially_destructible_input_iterator& operator++() {
return *this;
}
void operator++(int) {}
int operator*() const {
return 0;
}
bool operator==(default_sentinel_t) const {
return true;
}
};

using Inner = ranges::subrange<non_trivially_destructible_input_iterator, default_sentinel_t>;

auto r = views::empty<Inner> | views::join;
(void) r.begin();
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved

// Also validate _Non_propagating_cache
auto r2 = views::empty<Inner> | views::transform([](Inner& r) { return r; }) | views::join;
}

int main() {
// Validate views
constexpr string_view expected = "Hello World!"sv;
Expand Down
14 changes: 14 additions & 0 deletions tests/std/tests/P0896R4_views_single/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,20 @@ static_assert(same_as<ranges::single_view<const char*>, decltype(views::single("
void double_function(double);
static_assert(same_as<ranges::single_view<void (*)(double)>, decltype(views::single(double_function))>);

// Validate that the _Copyable_box primary template works when fed with a non-trivially-destructible type
void test_non_trivially_destructible_type() { // COMPILE-ONLY
struct non_trivially_destructible {
non_trivially_destructible() = default;
~non_trivially_destructible() {}

// To test the correct specialization of _Copyable_box, this type must not be copy assignable.
non_trivially_destructible(const non_trivially_destructible&) = default;
non_trivially_destructible& operator=(const non_trivially_destructible&) = delete;
};

(void) views::single(non_trivially_destructible{});
}

int main() {
static_assert(test_one_type(42, 42));
test_one_type(42, 42);
Expand Down