Overly permissive constructor

[improbablejan]

Hi, we have got a bunch of code in our repository that looks like this (simplified):

#include "span.hpp"
#include <cstddef>
#include <cstdint>

void foo(nonstd::span<std::uint8_t>) {}
void foo(nonstd::span<nonstd::span<std::uint8_t>>) {}

int main() {
  std::uint8_t u;
  foo({&u, 1u});
  return 0;
}

I am looking at upgrading our dependency, and while this used to work fine for us previously (using commit 874fa1b14e75d8ad45f28250caeb518687990c70), this no longer works using the release 0.9.0 as the call to foo is now ambiguous.

$ clang++-12 --std=c++11 span.cc 
span.cc:10:3: error: call to 'foo' is ambiguous
  foo({&u, 1u});
  ^~~
span.cc:5:6: note: candidate function
void foo(nonstd::span<std::uint8_t>) {}
     ^
span.cc:6:6: note: candidate function
void foo(nonstd::span<nonstd::span<std::uint8_t>>) {}
     ^
1 error generated.

This code also seems to work fine with gcc and clang's implementation of std::span, see https://gcc.godbolt.org/z/Y657dh

We presume this is due to some constructors being overly permissive and participating in overload resolution where they should not, but we have not looked into which particular one is the issue and this is just a hypothesis, so apologies if the title is inaccurate. I haven't checked the standard in detail to understand if this is supposed to work or not, but presume that the ambiguity is due to an oversight rather than by the standard - apologies if that's not the case!

[martinmoene]

Thanks @improbablejan , I've made a start looking into it. Don't know how quick I can solve it though.

[improbablejan]

Thanks for the quick fix and the great work @martinmoene ! We use a few of your libraries and it's been great to have access to modern C++ datatypes from C++11.

[martinmoene]

@improbablejan Thank you 😊

[improbablejan]

Hi @martinmoene , I tried taking the new release you created, but it breaks a lot of our existing code. In particular, https://github.com/martinmoene/span-lite/commit/387dc581265285492cdd5c2cf7c7ffe3b9a3fc31#diff-b6fd5b6d9ee6e564f9b2e7a44f2486fb451932cc0b4775cd0920da9582435273R923 seems to be the issue as it no longer accepts the iterator and count when the range is empty. Is this correct? https://en.cppreference.com/w/cpp/container/span/span says the constructor requires [first, first + count) to be a valid range, but it looks like your code is more restrictive? E.g. I would have expected the following code to work fine, but my understanding is that it is not valid anymore with your implementation:

std::vector<std::size_t> v;
nonstd::span<std::size_t> s(v.begin(), v.size());

[martinmoene]

Oh no... I'll take a look...

[martinmoene]

@improbablejan Had a look at what you describe. Can however not reproduce the problem with C++11 and later by compiling

std::vector<std::size_t> v;
nonstd::span<std::size_t> s(v.begin(), v.size());

Can you give some information on OS, compiler, C++standard (and perhaps other relevant info) you're using?

[improbablejan]

Of course. I am using this snippet:

#include "span.hpp"
#include <cstddef>
#include <vector>

int main() {
  std::vector<std::size_t> v;
  nonstd::span<std::size_t> s(v.begin(), v.size());
  return 0;
}

Version information below (using the 0.9.1 release):

$ clang++-12 --std=c++11 span.cc && ./a.out 
terminate called without an active exception
Aborted (core dumped)
$ clang++-12 --version
Ubuntu clang version 12.0.0-++20210303094403+9760b282ff03-1~exp1~20210303205143.53
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 18.04.5 LTS
Release:    18.04
Codename:   bionic

I also tried gcc 5 and 7 locally, and it's failing on all platforms in our CI. Sorry, I failed to mention that this causes a runtime, rather than compile time issue.

[martinmoene]

Ah, thanks :)

[martinmoene]

Ok, nonstd::span<std::size_t> s(v.begin(), v.size()); leads to constructor span(It, size_t) chosen, instead of span(It, End)...

[Historical note: Well, I was clearly confused, didn't notice the v.size() above]

[martinmoene]

Still needs more work... (cannot dereference end iterator/sentinel).

Under MSVC,

std::vector<int> v;
span<int> s( v.begin(), v.end() );

Leads to call to it.operator->() in to_address():

static inline span_constexpr pointer to_address( Ptr const & it ) span_noexcept
{
    return to_address( it.operator->() );
}