Details
- Reviewers
ldionne - Group Reviewers
Restricted Project - Commits
- rG775caa58fcf9: [libc++] [c++2b] [P2166] Prohibit string and string_view construction from…
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
libcxx/test/std/strings/basic.string/string.cons/nullptr.fail.cpp | ||
---|---|---|
18 ↗ | (On Diff #361680) | (1) I think we should also test at least one of the implicit-conversion syntaxes: std::string s = nullptr; and/or void f(std::string); f(nullptr); static_assert(!std::is_convertible_v<decltype(nullptr), std::string>); static_assert(!std::is_constructible_v<std::string, decltype(nullptr)>); static_assert(!std::is_assignable_v<std::string, decltype(nullptr)>); and leave it at that. I was going to suggest an overload-resolution test like void f(std::unique_ptr<int>) { } void f(const std::string&); int main() { f(nullptr); } which I'm sure programmers would like to Just Work... but [p2166] deliberately keeps that as ambiguous. (The user-defined conversion from nullptr to std::string ranks lower in preference than a standard conversion such as nullptr-to-int*, but delete'ing it doesn't make it go away; it just says "The API designer knows what you're trying to do, and you're wrong to try it." So we still have an ambiguity here between the right thing (conversion to unique_ptr) and the wrong thing (conversion to string), with no tiebreaker for picking between them.) |
libcxx/test/std/strings/basic.string/string.cons/nullptr.fail.cpp | ||
---|---|---|
18 ↗ | (On Diff #361680) | I would go for the static_assert tests in a .compile.pass.cpp test file too. All in the same file IMO. |
Requesting changes so it shows up right in the review queue, but feel free to ping me on Discord for a pretty trivial review once you update this and CI passes.
LGTM but please remove main()s!
libcxx/test/std/strings/string.view/string.view.cons/nullptr.compile.pass.cpp | ||
---|---|---|
23 | You don't need to define a main() for compile.pass.cpp tests. Not defining a main makes it clear that the test was never intended to be run. Applies above too. |
You don't need to define a main() for compile.pass.cpp tests. Not defining a main makes it clear that the test was never intended to be run. Applies above too.