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

Visibility bug in gcc-9 and below using concepts #1546

Closed
marehr opened this issue Aug 10, 2020 · 1 comment
Closed

Visibility bug in gcc-9 and below using concepts #1546

marehr opened this issue Aug 10, 2020 · 1 comment

Comments

@marehr
Copy link
Contributor

marehr commented Aug 10, 2020

The following code has unwanted side effects:

#include <range/v3/view/split.hpp>

class A {
  int i{5};
};

int main()
{
    return A{}.i; // should be illegal, because it is private
}

https://godbolt.org/z/rP1bGK

which makes any private/protected member public.

Expected output:

<source>: In function 'int main()':
<source>:9:16: error: 'int A::i' is private within this context
    9 |     return A{}.i; // should be illegal, because it is private
      |                ^
<source>:4:7: note: declared private here
    4 |   int i{5};
      |       ^

This is a known gcc-bug which was fixed in gcc-10. We internally have the following concept check to make sure we don't use any concept syntax that triggers this bug:

template <typename t> concept private_bug = requires(t a) { a.i; };
static_assert(!private_bug<A>, "See https://github.com/seqan/seqan3/issues/1317");

(I love this expression, because you can have self-warning code for concepts with concepts xD)


I tried to reduce the code and this came out:

#include <utility>
namespace ranges {
template <typename I1, typename I2 = I1> concept indirectly_swappable = true;
}
namespace ranges {
template <typename Rng>
using iterator_t = decltype(begin(std::declval<Rng &>()));
template <typename V, typename Pattern> struct split_view;
namespace detail {
template <typename JoinView, bool Const> struct split_inner_iterator;
template <typename V, typename Pattern, bool Const>
struct split_inner_iterator<split_view<V, Pattern>, Const> {
  using Base = V;
// <<<<<<<<<<<<<<<<<<<<<<<< misbehaving code
  template <bool = false>
  friend constexpr auto iter_swap(split_inner_iterator const &x,
                                  split_inner_iterator const &y)
      -> std::enable_if_t<indirectly_swappable<iterator_t<Base>> && true> {}
// >>>>>>>>>>>>>>>>>>>>>>>>
};
} // namespace detail
} // namespace ranges
class A {
  int i{5};
};
template <typename t> concept private_bug = requires(t a) { a.i; };
static_assert(!private_bug<A>,  "See https://github.com/seqan/seqan3/issues/1317");

https://godbolt.org/z/rP1bGK

Adding parenthesis around the && expression, solves this issue.

// <<<<<<<<<<<<<<<<<<<<<<<< misbehaving code
  template <bool = false>
  friend constexpr auto iter_swap(split_inner_iterator const &x,
                                  split_inner_iterator const &y)
      -> std::enable_if_t<(indirectly_swappable<iterator_t<Base>> && true)> {}
// >>>>>>>>>>>>>>>>>>>>>>>>
@marehr
Copy link
Contributor Author

marehr commented Aug 10, 2020

b52e3b5 is the first bad commit
commit b52e3b5
Author: Eric Niebler eniebler@boostorg
Date: Mon Aug 3 19:52:55 2020 -0500

Rework the concepts emulation later to do short-circuiting in requires clauses (#1539)

ericniebler added a commit that referenced this issue Aug 14, 2020
fix #1546: Visibility bug in gcc-9 and below using concepts
marehr added a commit to marehr/seqan3 that referenced this issue Aug 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant