Skip to content

Conversation

@arrowd
Copy link
Contributor

@arrowd arrowd commented Nov 11, 2025

This also fixes compilation with EDG-based C++ compilers:

error 289: no instance of constructor
"std::span<_Type, _Extent>::span [with _Type=std::byte, _Extent=18446744073709551615UL]"
matches the argument list

@MaxKellermann
Copy link
Member

The description is lacking.

  1. it says it's a "simplification" but it does not actually simplify the initializer; it removes it completely.
  2. does not tell why removing the initializer is valid
  3. does not tell what the error is about (what even is an "EDG-based C++ compiler"?)

@arrowd
Copy link
Contributor Author

arrowd commented Nov 11, 2025

it says it's a "simplification" but it does not actually simplify the initializer; it removes it completely.

The span still gets initialized, right? It is done by its trivial constructor now.

does not tell why removing the initializer is valid

I baffled why removing an invalid initializer should be justified, not the reverse. Out of all span constructors listed at https://en.cppreference.com/w/cpp/container/span.html which one can be passed with {nullptr}?

does not tell what the error is about

I quoted the compiler's error message in the commit's description. What's unclear there?

(what even is an "EDG-based C++ compiler"?)

https://edg.com/c

@MaxKellermann
Copy link
Member

it says it's a "simplification" but it does not actually simplify the initializer; it removes it completely.

The span still gets initialized, right? It is done by its trivial constructor now.

Your commit message should say that.

I baffled why removing an invalid initializer should be justified, not the reverse. Out of all span constructors listed at https://en.cppreference.com/w/cpp/container/span.html which one can be passed with {nullptr}?

Your commit message does not explain that the code is invalid. It only says it "simplifies" an initializer (which it does not) and cites the error by a C++ compiler that, by the description on its own home page, is not C++23 compliant. This sounds like you want to work around a compiler bug, because your commit message does not claim that MPD code is bad. If you claim that MPD code is bad and should be fixed, your commit message should describe that.

Text posted here does not count. The commit message should say it.

@MaxKellermann
Copy link
Member

It is done by its trivial constructor now.

Nit: it's the default constructor. And this default constructor is, by definition, not trivial.

@arrowd
Copy link
Contributor Author

arrowd commented Nov 11, 2025

Hmm, I'm a bit confused now. The minified example

#include <span>

struct X {
  std::span<std::byte> buffer{nullptr};
};

fails to compile in Clang 19 too:

% c++ t.cpp -std=c++26
t.cpp:4:30: error: no matching constructor for initialization of 'std::span<std::byte>'
    4 |   std::span<std::byte> buffer{nullptr};
      |                              ^~~~~~~~~

... but it somehow works when compiling MPD.

I'll try to amend the commit message as much as I can, though.

@MaxKellermann
Copy link
Member

... but it somehow works when compiling MPD.

The default constructor is never called in MPD and thus never instantiated. A template that is never instantiated will not throw this compiler error.

I believe the strict behavior of EDG is a compiler bug. EDG cannot prove that a certain template overload of std::span does not have a constructor accepting nullptr.

We do know that. The MPD code is bad, it's a MPD bug, but one that never materializes. It's in dead code, therefore it has no practical relevance. It is only a theoretical bug. Yet your commit message failed to describe this and is insufficient (and even wrong).

It is invalid to initialize a std::span<std::byte> with nullptr. The code was
compiling fine, however, because most compilers do not try to instantiate
this template, unless it is called somewhere in the code.
@arrowd
Copy link
Contributor Author

arrowd commented Nov 11, 2025

Is this better?

@MaxKellermann MaxKellermann merged commit 7932faf into MusicPlayerDaemon:master Nov 11, 2025
7 checks passed
@MaxKellermann
Copy link
Member

Yes!

@arrowd arrowd deleted the std-span-init-fix branch November 11, 2025 18:19
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

Successfully merging this pull request may close these issues.

2 participants