Currently, span overloads on nsTArray, such as AppendElements, always take Span<const Item>. This unfortunately can cause build errors when you attempt to pass a span which contains non-const qualified members, such as Span<RefPtr<BrowsingContext>>.

Relaxes the type constraints for span overloads on nsTArray, such as
AppendElements. They previously took Span<const Item>, which could cause
build errors when attempting to pass a non const-qualified Item such as in
Span<RefPtr<BrowsingContext>>.

Or should Span have an implicit constructor that allows these safe conversions?
std::span does it, see 8th constructor in https://en.cppreference.com/w/cpp/container/span/span

(But I see you already have a patch that you probably need right now, so this suggestion could be done in another bug...)

Henri, does this change have no problem?

I expect this to have a problem when passing Span<const Item>, which is intended to work. The test that is named like it tests that case doesn't appear to test that case. Please make the test actually test that case to show that it works. If that case actually works due to the magic of C++, then OK.

For clarity, I don't object to adding support for passing Span<Item> as long as support for passing Span<const Item> continues to work.

(In reply to Gerald Squelart [:gerald] (he/him) from comment #2)

Or should Span have an implicit constructor that allows these safe conversions?
std::span does it, see 8th constructor in https://en.cppreference.com/w/cpp/container/span/span

(But I see you already have a patch that you probably need right now, so this suggestion could be done in another bug...)

In principle, it should be OK to pass Span<Foo> to something that takes Span<const Foo>, and it should just work. I assume that the refcount changing is the const-correctness problem here. (The C++ standard library design doesn't acknowledge Gecko/WebKit/Gtk-style interior refcounts.) Nika, is that the problem?

(In reply to Henri Sivonen (:hsivonen) from comment #5)

For clarity, I don't object to adding support for passing Span<Item> as long as support for passing Span<const Item> continues to work.

It does. I fixed the test in my patch, and it continues to pass. const Foo is supported here because the Item type is a template parameter of AppendElements, and thus can be substituted be const Foo, rather than being tied to the type of nsTArray itself.

(In reply to Henri Sivonen (:hsivonen) from comment #6)

(In reply to Gerald Squelart [:gerald] (he/him) from comment #2)

Or should Span have an implicit constructor that allows these safe conversions?
std::span does it, see 8th constructor in https://en.cppreference.com/w/cpp/container/span/span

In principle, it should be OK to pass Span<Foo> to something that takes Span<const Foo>, and it should just work. I assume that the refcount changing is the const-correctness problem here. (The C++ standard library design doesn't acknowledge Gecko/WebKit/Gtk-style interior refcounts.) Nika, is that the problem?

We should definitely have an implicit conversion constructor there, and assuming that we act as much like std::span as possible, it will behave correctly in non-generic contexts. The problem with nsTArray is that the Item type is actually a free template type on AppendElements, and thus has nothing to do with the actual item type.

This causes conversion guides, like a Span implicit constructor, to not be attempted, and means that the conversion will fail. Here's a godbolt using c++20's std::span which demonstrates the difference here:

https://godbolt.org/z/LwRNUo

#include <span>

template <typename T>
struct nsTArray {
  template <typename Item>
  T* GenericMethod(std::span<const Item> aSpan) {
    return nullptr;
  }

  T* NongenericMethod(std::span<const T> aSpan) {
    return nullptr;
  }
};

int main() {
  int arr[] = {1, 2, 3, 4};
  std::span<int> span{arr};
  std::span<const int> constSpan{arr};

  nsTArray<int> array;
  array.GenericMethod(span);         // Build Error
  array.GenericMethod(constSpan);    // OK

  array.NongenericMethod(span);      // OK
  array.NongenericMethod(constSpan); // OK
}

https://hg.mozilla.org/mozilla-central/rev/fc49269ac8a6