Created attachment 61498 [details]
preprocessed source

clang 19 says

> clang-19 t.C -S -std=c++20
t.C:4:11: error: no viable overloaded '-='
    4 |     __tmp -= __n
      |     ~~~~~ ^  ~~~
t.C:4:17: error: expected ';' after expression
    4 |     __tmp -= __n
      |                 ^
      |                 ;

so this possibly was accepts-invalid before?

taken from openmsx 20.0

Created attachment 61499 [details]
CommandLineParser.C.xz

Partially unincluded testcase (with libstdc++ and glibc headers #included, tcl and SDL2 preprocessed).

Bisected to r14-11483-g5a830c6cd54d376ee23043381c6ed761559e1e08
Now the question is if openmsx does something invalid or libstdc++ relies on something not guaranteed, and even if the former, whether the change is appropriate for release branch or not.

I suspect this is a similar problem to Bug 120325 i.e. an iterator type which is not compatible with C++20 rules, and needs to be fixed or opt-out of being a C++20 random access iterator.

I think this is on
using GroupedItems = hash_map<string_view, std::vector<string_view>, XXHasher>;
static void printItemMap(const GroupedItems& itemMap)
{
 auto printSet = to_vector(view::transform(itemMap, [](auto& p) {
  return strCat(formatSet(p.second, 15), ' ',
                formatHelpText(p.first, 50, 20));
 }));
 ranges::sort(printSet);
 for (const auto& s : printSet) {
  cout << s << '\n';
 }
}

(In reply to Richard Biener from comment #2)
> clang 19 says
> 
> > clang-19 t.C -S -std=c++20
> t.C:4:11: error: no viable overloaded '-='
>     4 |     __tmp -= __n
>       |     ~~~~~ ^  ~~~
> t.C:4:17: error: expected ';' after expression
>     4 |     __tmp -= __n
>       |                 ^
>       |                 ;
> 
> so this possibly was accepts-invalid before?

Comment 0 is just a tiny piece of libstdc++ code taken out of context, and so is not valid code, and is nothing to do with the bug.

The error I see for the full CommandLineParser.ii file is this:

src/CommandLineParser.cc:423:27:   required from here
src/utils/view.hh:210:15: error: no match for 'operator-' (operand types are 'const hash_set<std::pair<std::basic_string_view<char>, std::vector<std::basic_string_view<char> > >, hash_set_impl::ExtractFirst, XXHasher, std::equal_to<void> >::Iter<const hash_set<std::pair<std::basic_string_view<char>, std::vector<std::basic_string_view<char> > >, hash_set_impl::ExtractFirst, XXHasher, std::equal_to<void> >, const std::pair<std::basic_string_view<char>, std::vector<std::basic_string_view<char> > > >' and 'const hash_set<std::pair<std::basic_string_view<char>, std::vector<std::basic_string_view<char> > >, hash_set_impl::ExtractFirst, XXHasher, std::equal_to<void> >::Iter<const hash_set<std::pair<std::basic_string_view<char>, std::vector<std::basic_string_view<char> > >, hash_set_impl::ExtractFirst, XXHasher, std::equal_to<void> >, const std::pair<std::basic_string_view<char>, std::vector<std::basic_string_view<char> > > >')

This is a completely different error coming from the openmsx code.

Their hash_set provides an iterator type which meets the C++17 forward iterator requirements:

template<typename Value,
         typename Extractor = std::identity,
         typename Hasher = std::hash<hash_set_impl::ExtractedType<Value, Extractor>>,
         typename Equal = std::equal_to<>>
class hash_set
{
protected:
 using PoolIndex = hash_set_impl::PoolIndex;
 static constexpr auto invalidIndex = hash_set_impl::invalidIndex;

public:
 using value_type = Value;

 template<typename HashSet, typename IValue>
 class Iter {
 public:
  using value_type = IValue;
  using difference_type = int;
  using pointer = IValue*;
  using reference = IValue&;
  using iterator_category = std::forward_iterator_tag;

And then they provide a TransformIterator adaptor to wrap other iterators and provide extra functionality. The problem is the TransformIterator adaptor which has all these completely unconstrained operators:

 constexpr TransformIterator& operator+=(difference_type n)
 {
  it += n;
  return *this;
 }

 constexpr TransformIterator& operator-=(difference_type n)
 {
  it -= n;
  return *this;
 }

 [[nodiscard]] constexpr friend TransformIterator operator+(TransformIterator x, difference_type n)
 {
  x += n;
  return x;
 }
 [[nodiscard]] constexpr friend TransformIterator operator+(difference_type n, TransformIterator x)
 {
  x += n;
  return x;
 }

 [[nodiscard]] constexpr friend TransformIterator operator-(TransformIterator x, difference_type n)
 {
  x -= n;
  return x;
 }

 [[nodiscard]] constexpr friend difference_type operator-(const TransformIterator& x, const TransformIterator& y)
 {
  return x.it - y.it;
 }

 [[nodiscard]] constexpr reference operator[](difference_type n)
 {
  return *(*this + n);
 }

 [[nodiscard]] constexpr auto operator<=>(const TransformIterator& other) const
 {
  return it <=> other.it;
 }


The C++20 library sees these and thinks that it must be a random access iterator, because it provides all the operations that a random access iterator provides. But when it tries to use those operations on a TransformIterator<hash_set::Iter> they fail to compile.

This code is not ready to be compiled as C++20.

There are a few ways to fix it, they could add a requires-clause (or enable_if constraint) to each of those operators in TransformIterator, so they can only be used if the underlying iterator being adapted supports it.

Or they could define TransformIterator::iterator_concept to stop the C++20 library trying to deduce that for itself.

Or they could specialize std::sized_sentinel_for for TransformIterator.

I'm testing those now ...

There's a one-line fix:

  [[nodiscard]] constexpr friend difference_type operator-(const TransformIterator& x, const TransformIterator& y)
+   requires std::sized_sentinel_for<Iterator, Iterator>
  {
   return x.it - y.it;
  }


This disables operator- for TransformIterator if the underlying iterator doesn't support it, and that's enough to prevent the C++20 library trying to use TransformIterator as a C++20 random access iterator.

Since this code attempts to be valid C++20 (e.g. it defines operator<=> for Transformiterator) it REALLY needs to do this.

Ideally they should constrained ALL the operators on that adaptor.

(In reply to Jonathan Wakely from comment #6)
> I suspect this is a similar problem to Bug 120325 i.e. an iterator type
> which is not compatible with C++20 rules, and needs to be fixed or opt-out
> of being a C++20 random access iterator.

And yes, this is exactly the problem.

C++20 has a lot of new rules, code that uses -std=c++20 needs to follow them.

But changing the library semantics between 14.2.0 and 14.3.0 isn't ideal ...

Upstream already replaced their transform view with std::views::transform so probably aren't going to be interested in the patch:

https://github.com/openMSX/openMSX/commit/e4aa15cbfdfb3e1b94ae7e787f2be0fbf44eaa75

Invalid then.

I've added a note to the bottom of https://gcc.gnu.org/gcc-14/porting_to.html#cxx20-iterators which links to https://gcc.gnu.org/gcc-15/porting_to.html#cxx20-iterators