[Bug c++/107361] Why does -Wclass-memaccess require trivial types, instead of trivially-copyable types?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107361 Peter Kasting changed: What|Removed |Added CC||pkasting at google dot com --- Comment #3 from Peter Kasting --- This warning fired in Chromium on safe code that clang doesn't warn about, for similar reasons as the reporter here complains about. I think the linked patch was too aggressive. At the very least, `memcpy()` from some instance `t1` of a `T` into another instance `t2` of a `T` should only require that `T` is trivially-copyable. Since we can assume that `t1`'s invariants held before the statement, after it `t2`'s invariants must hold (or the coder should not have allowed trivial copyability, which is a distinct bug). I do agree with the original patch author that `memset()` probably requires the type to be trivial rather than just trivially-copyable. We can work around with cast-to-`void*` prior to `memcpy()`, which feels unfortunate given that the intent of that is less obvious to a reader, and it's probably easier to introduce dangerous behavior later.
[Bug libstdc++/113074] std::to_address should be SFINAE safe
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113074 --- Comment #17 from Peter Kasting --- (In reply to Jonathan Wakely from comment #15) > (In reply to Peter Kasting from comment #14) > > And you are right, it's possible to reimplement concepts around "is this > > even legal to pass to to_address()", which is basically what we're doing to > > address this in Chromium. But that's pretty unfriendly to most usage; if > > you're in a context where you are reaching for to_address() to begin with, > > it's likely because you're templated and don't know that a simpler thing > > like `&*ptr` is valid. > > The reason to avoid &*ptr is because it's undefined behaviour on a > past-the-end iterator, not because it might be ill-formed for some template > argument types. That's the reason to avoid it in the specific case the library happened to make use of std::to_address(). That's not necessarily reasoning that applies to other uses of std::to_address(). > > That's fair, but isn't that implementable by simply making the definition of > > contiguous_iterator explicitly hard error in this case? That is, an > > SFINAE-friendly to_address() wouldn't prevent you from diagnosing this > > particular usage site as incorrect. > > The rationale for making it SFINAE-friendly in libc++ was to *not* give an > error for that case. Making it SFINAE-friendly but restoring the error in > that case would remove the reason to have made it SFINAE-friendly in the > first place. I'm not sure how many more ways I can say that my interest in this being SFINAE-friendly is completely unrelated to libc++'s (or any other STL's) handling of contiguous_iterator. The committee added a tool, used it one place, made it hard to hold other places, and (per your previous summaries) has avoided making it more useful other places due to only thinking about things through the lens of this specific contiguous_iterator usage. I'm not using it for that. > If you think this should change, please take that through WG21. I don't > think we should deviate from the standard here. Yes, precisely. Per comment 8: "libstdc++ is standards-compliant as-is. We'll fix our code. I do think libc++'s behavior is more usable and an LWG issue would be nice." I don't have any connection to WG21. I was hoping someone here had the experience necessary to pass on this request to the committee. If not, so be it, it will die.
[Bug libstdc++/113074] std::to_address should be SFINAE safe
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113074 --- Comment #14 from Peter Kasting --- (In reply to Jonathan Wakely from comment #13) > As I said in comment 7, LWG considered this case and it was pointed out that > the problem described can only occur if a type defines iterator_concept = > contiguous_iterator; but then fails to actually provide the operations > needed for a contiguous iterator (i.e. either a pointer_traits > specialization with to_address or a sane operator->()). That's fair. But that only considers the functionality of to_address() inside this specific library use of it. If this tool is to be usable in other contexts (which I argue it should be, or it shouldn't have been exposed to end users), then said contexts may have nothing to do with iterators. And you are right, it's possible to reimplement concepts around "is this even legal to pass to to_address()", which is basically what we're doing to address this in Chromium. But that's pretty unfriendly to most usage; if you're in a context where you are reaching for to_address() to begin with, it's likely because you're templated and don't know that a simpler thing like `&*ptr` is valid. In such cases, having to_address() be SFINAE-friendly makes it far easier to fall back to other handling for "not a pointer". > A SFINAE-friendly std::to_address as implemented in libc++ means that such > an iterator will fail to satisfy std::contiguous_iterator and will silently > degrade to satosfying std::random_access_iterator only. It's not at all > clear to me that silently degrading such an iterator (which very explicitly > claims to be a contiguous iterator by defining iterator_concept to say so) > would be an improvement. I'd rather get an error telling me the thing I > thought was a contiguous iterator was not actually. That's fair, but isn't that implementable by simply making the definition of contiguous_iterator explicitly hard error in this case? That is, an SFINAE-friendly to_address() wouldn't prevent you from diagnosing this particular usage site as incorrect.
[Bug libstdc++/113074] std::to_address should be SFINAE safe
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113074 --- Comment #10 from Peter Kasting --- (In reply to Jonathan Wakely from comment #9) > (In reply to Andrew Pinski from comment #5) > > Created attachment 56905 [details] > > testcase which shows libc++ and libstdc++ difference > > > > with libstdc++, both GCC and clang reject this. > > with libc++, clang accepts this (I only tested clang because it was the > > easiest to test there). > > AFAICT only libc++ from llvm trunk accepts this, even 17.0.1 gives an error > outside the immediate context, like libstdc++ does. Yes, but that was because libc++ did not implement LWG3545 until LLVM 18. When Jose reported this issue in Chromium I pushed back on fixing on our side because I thought libstdc++ had the same issue. But this is a distinct issue; std::to_address<> is not required to be SFINAE-compatible even if std::pointer_traits<> now is.
[Bug libstdc++/113074] std::to_address should be SFINAE safe
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113074 Peter Kasting changed: What|Removed |Added CC||pkasting at google dot com --- Comment #8 from Peter Kasting --- As a Chromium C++ OWNER -- agree that libstdc++ is standards-compliant as-is. We'll fix our code. I do think libc++'s behavior is more usable and an LWG issue would be nice. There's currently no simple and blessed route for consumers to convert raw-or-fancy-pointer input types to raw pointers.
[Bug libstdc++/109242] C++2b std::optional::transform omits required std::remove_cv_t from return optional type
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109242
--- Comment #2 from Peter Kasting ---
(In reply to TC from comment #1)
> The missing remove_cv_t is real, but this example is invalid. As the linked
> cppreference page notes, you cannot pass a PMD to transform.
Ah, true! How about this then: https://godbolt.org/z/TaccPEsY1
#include
struct S {
int& i() const;
};
void foo() {
std::optional().transform(::i);
}
[Bug libstdc++/109242] New: C++2b std::optional::transform omits required std::remove_cv_t from return optional type
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109242
Bug ID: 109242
Summary: C++2b std::optional::transform omits required
std::remove_cv_t from return optional type
Product: gcc
Version: 13.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: libstdc++
Assignee: unassigned at gcc dot gnu.org
Reporter: pkasting at google dot com
Target Milestone: ---
The implementation of C++2b's std::optional::transform() on HEAD omits a call
to std::remove_cv_t when determining the value type of the returned optional.
As a result valid code such as the following example is rejected:
https://godbolt.org/z/hG5hnz59e
#include
struct S {
int i;
};
void foo() {
std::optional().transform(::i);
}
Per https://en.cppreference.com/w/cpp/utility/optional/transform, the returned
type should always be computed via std::remove_cv_t<> on the invoke_result, but
per
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/include/std/optional;hb=HEAD,
_Up consistently omits this.
