P2166R1 Prohibiting basic_string And basic_string_view Construction From nullptr

[sam20908]

Fixes #1979

[sam20908]

One question regarding consistency.

[miscco]

Also while you are in the vicinity, would you be interested in #1977

[sam20908]

Also while you are in the vicinity, would you be interested in #1977

Yeah I'm interested to tackle that too if that's available.

[CaseyCarter]

Also: I added an entry to the yvals_core.h list of implemented proposals in a new _HAS_CXX23 directly controls: section. (It seemed easier to just push a commit than to try to explain the proper place since we don't have a _HAS_CXX23 section yet.)

[statementreply]

Is this supposed to be applied retroactively, or to C++23 only? cplusplus/papers#887 is tagged "C++20".

[StephanTLavavej]

This PR needs to merge main and resolve a conflict in yvals_core.h.

[SuperWig]

GitHub's merge conflict editor doesn't like this repo's line endings.

Is this supposed to be applied retroactively, or to C++23 only?

IMO it should go all the way back to C++11. The fact that this compiles is super dumb,

#include <string>

std::string boom()
{
    return 0;
}

[sam20908]

Merge finally fixed...

[SuperWig]

yvals_core.h still has mixed line endings. It should be CRLF.

[StephanTLavavej]

Looks great! I'll validate and push trivial changes to the test.

[StephanTLavavej]

@statementreply

Is this supposed to be applied retroactively, or to C++23 only? cplusplus/papers#887 is tagged "C++20".

Good question. Looking at that issue's history, it appears that it was tagged C++20 because it was considered for retroactive application - however, it appears that they decided against that. The final motion that was sent to the plenary vote was for C++23 only.

@SuperWig

IMO it should go all the way back to C++11.

I agree that this detects totally bad code, although I'd be slightly concerned about customers complaining that this causes compiler errors for code that isn't executed at runtime. It does seem almost like [[nodiscard]] (warning/erroring about extremely questionable code), although our classic criterion for implementing a feature unconditionally is when it poses a low risk of breakage and would be difficult to disentangle - here it's very easy to guard.

[StephanTLavavej]

Thanks for implementing this C++23 feature which has already found incorrect code in the compiler! 😹

[jwakely]

Good question. Looking at that issue's history, it appears that it was tagged C++20 because it was considered for retroactive application - however, it appears that they decided against that. The final motion that was sent to the plenary vote was for C++23 only.

No, adding the C++20 label there appears to just be an admin error in the paper tracker. This paper was never in scope for C++20 and never intended to be a DR against C++20.

IMO it should go all the way back to C++11.

That's what I did for GCC.

I agree that this detects totally bad code, although I'd be slightly concerned about customers complaining that this causes compiler errors for code that isn't executed at runtime.

I got a number of reports like that, but everybody agreed to fix their bad code. Nobody argued "it's OK, we never actually run this undefined code, we want to keep it!"

The real problem with making it apply back to C++11 is it changes previously-correct code to ill-formed:

#include <string>

struct S
{
  operator const char*() const { return ""; }
  operator std::nullptr_t() const { return {}; }
};

std::string s{ S{} };

This is valid in C++20 and earlier, but ill-formed in C++23. GCC rejects this in C++20, which I suppose is a bug.

[CaseyCarter]

This is valid in C++20 and earlier, but ill-formed in C++23. GCC rejects this in C++20, which I suppose is a bug.

It's hard for me not to see a type that converts to both nullptr_t and to pointer type with possibly non-null value as broken regardless of this issue. By all means, convert to multiple different types, but if any of those types share a common type those conversions should be consistent.

[jwakely]

No argument here. I think the json issue was actually something more like this, where there's a variant-like type that can be implicitly converted to various types:

struct J
{
  // In C++20 this selects basic_string(const char*),
  // in C++23 it's ambiguous due to basic_string(nullptr_t).
  template<typename T>
    requires (!std::is_same_v<std::allocator<char>, T>)
    && (!std::is_same_v<std::string, T>)
    && (!std::is_same_v<char, T>)
    && (!std::is_same_v<std::string_view, T>)
    operator T() const { return {}; }
};

std::string s3{ J{} };

And this seems extremely fragile too.