Provide reasonable default definition for is_contiguous#4731
Provide reasonable default definition for is_contiguous#4731user202729 wants to merge 1 commit intofmtlib:mainfrom
Conversation
8b0fd20 to
c9a57d4
Compare
c9a57d4 to
5f956f9
Compare
vitaut
left a comment
There was a problem hiding this comment.
Thanks for the PR! The idea makes sense but let's move SFINAE from a helper function and varargs hack to is_contiguous using void_t.
|
Requiring |
|
I agree that we should drop |
|
Two technical concerns with this approach: 1.
2. Varargs overload is unprincipled
Your suggestion to use |
Another attempt at #4716 . Compiling
base-test.cctakes around 10 seconds (both before and after), probably is fine. (not sure what else to measure.)Instead of using
contiguous_iterator, I just check whether.data()can be called on it; if it can be called, the container is likely contiguous. Opinion?Some code assumes that if it's contiguous,
.resize()exists, so it won't work for e.g.std::arrayeven though that one is contiguous. But thenback_insert_iteratordoesn't work onstd::arrayeither.Directly define a partial specialization of
is_contiguouswithtypename = voidleads to ambiguous overload errors, which is backwards incompatible if someone else special-caseis_contiguous. See for example https://github.com/user202729/fmt/actions/runs/24015836992/job/70035092887