This should be clearer, instead of relying on rules for implicit
conversions regarding built in float/integer types.
Details
- Reviewers
cjdb • Quuxplusone curdeius ldionne - Group Reviewers
Restricted Project - Commits
- rG91d6debbb913: [libcxx] [test] Use dedicated types for the invocable concept tests for…
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I think that now we test less than before. Precisely, this code tested before that e.g. one could invoke f(double) with a float argument.
I'd like to see:
struct DA : A {}; struct DB : B {}; static_assert(std::regular_invocable<multiple_overloads&, multiple_overloads::DA>); static_assert(std::regular_invocable<multiple_overloads&, multiple_overloads::DB>);
(and the same for std::invocable)
Apart from that, it looks good.
That was the intent of my comment on the other review (and Arthur's original comment too) AFAICT. Thanks!
libcxx/test/std/concepts/concepts.callable/concept.invocable/invocable.compile.pass.cpp | ||
---|---|---|
240 | @curdeius wrote:
I had tested that codepath (that invocable deals correctly with implicit-conversions-to-the-parameter-type) by std::invocable<multiple_overloads, int> (int being implicitly convertible to B). @curdeius, any objection if I re-remove DA and DB? (If so: Any objection if I remove DB and B(int)? If so: I'll remove DB.) |
libcxx/test/std/concepts/concepts.callable/concept.invocable/invocable.compile.pass.cpp | ||
---|---|---|
240 | I agree that we don't need both DA and DB at the same time, the same way we don't need both A and B at the same time. |
@curdeius wrote:
I had tested that codepath (that invocable deals correctly with implicit-conversions-to-the-parameter-type) by std::invocable<multiple_overloads, int> (int being implicitly convertible to B).
There's certainly no reason to test all DA and DB and int — all three are exactly isomorphic, user-defined implicit conversions via non-explicit constructor.
@curdeius, any objection if I re-remove DA and DB? (If so: Any objection if I remove DB and B(int)? If so: I'll remove DB.)