[Bug libstdc++/90704] filesystem::path overloads for file streams are not conforming

[Bug libstdc++/90704] filesystem::path overloads for file streams are not conforming

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90704

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |manx-bugzilla@problemloesun
                   |                            |gsmaschine.de

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
*** Bug 95642 has been marked as a duplicate of this bug. ***

[Bug libstdc++/90704] filesystem::path overloads for file streams are not conforming

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90704

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
FWIW I opened https://cplusplus.github.io/LWG/issue3430 to say string_view
should be usable directly.

It makes absolutely no sense for construction from a string_view to convert to
filesystem::path just to add a null terminator to the end. It's not even clear
that we want implicit conversion from string_view to work.

[Bug libstdc++/90704] [LWG 3430] filesystem::path overloads for file streams are not conforming

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90704

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |SUSPENDED
            Summary|filesystem::path overloads  |[LWG 3430] filesystem::path
                   |for file streams are not    |overloads for file streams
                   |conforming                  |are not conforming

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I think I'm going to suspend this until the committee expresses a preferred
direction for the issue.

[Bug libstdc++/90704] [LWG 3430] filesystem::path overloads for file streams are not conforming

[manx-bugzilla at problemloesungsmaschine dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90704

--- Comment #5 from Jörn Heusipp <manx-bugzilla at problemloesungsmaschine dot de> ---

> Status: SUSPENDED

Well, coming from bug 95642, which has been marked as a duplicate of this bug,
this is *very* disappointing.


The C++17 standard absolutely clearly specifies that the constructor is
overloaded for std::filesystem::path. libstdc++'s SFINAE implementation is
incompatible because it fails for user-defined types that are implicitly
convertible to std::filesystem::path.
In order to at all be able to sensibly use libstdc++ std::filesystem::path in
my codebase, I would have to put this awful work-around (making tons of
assumptions about libstdc++'s enable_if expression) into my own type
mpt::PathString:
+       // work-around for GCC bug
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95642 /
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90704
+       mpt::PathString & make_preferred()
+       {
+               *this =
mpt::PathString::FromFsPath(std::filesystem::path{path}.make_preferred());
+               return *this;
+       }
+       mpt::PathString filename() const
+       {
+               return
mpt::PathString::FromFsPath(std::filesystem::path{path}.filename());
+       }
+       std::filesystem::path c_str() const
+       {
+               return std::filesystem::path{path};     
+       }
and just pray and hope that libstdc++ will not change its non-conforming
SFINAE-using implementation details to some other non-conforming implementation
details in some future version.
Can you guarantee, that you wont ever change your non-conforming SFINAE
implementation whatsoever at all? If not, please just fix it. The standard is
absolutely clear here.
If you insist on doing something non-standard-conforming because of
std::string_view, at the very least please do it for string_view only, without
breaking behaviour for std::filesystem::path. Compatibility with
std::experimental::filesystem::path can also be implemented without breaking
the standard-specified std::filesystem::path overloads.


The issue risen by https://cplusplus.github.io/LWG/issue3430 about the fear of
implicit allocation is not convincing to me. std::basic_fstream accesses the
filesystem, inducing all kinds of memory allocations, in-kernel blocking, and
disk I/O. Some additional user space allocations just do not matter here. None
of the call chain is specified noexcept either.
While the issue of desiring std::string_view overloads itself appears
reasonable to me, I fail to see why this issue should wait on that particular
standard change. Even if std::string_view overloads existed, the current
libstdc++ implementation would still be non-conforming for the
std::filesystem::path overload and would need to be changed in exactly the same
way as according to the current standard.


The suggestion of forcing std::filesystem::path arguments to be exactly
std::filesystem::path (i.e. making the current libstdc++ conforming by forcing
the bug on everyone else) breaks backwards compatibility for other (currently
conforming) implementations and IMO is also wrong on a higher level. There
exist legitimate and semantically sound reasons to have user-defined types that
are implicitly convertible to std::filesystem::path (std::filesystem::path's
greedy behaviour of silently doing enconding conversions from char (which may
be in locale encoding or utf8 in C++17, no way to tell) and thereby potentially
losing data, being one of them, making it impossible to use it safely by
itself). In the end, IMO std::filesystem::path should never have been
implicitly convertible from a string in the first place. *This* is where the
problems began. A string is not necessarily a path, which is why implicit
conversion was/is wrong here. I doubt that will ever change by now, though,
making it even more important to be able to wrap std::filesystem::path in an
encoding-type-safe user-defined type.


So, there are a couple of things wrong related to std::filesystem::path. The
only way to sanely use it, is by strictly adhering to the agreed-upon standard.
Delaying conformance with the standard until after some unrelated (to the
std::filesystem::path overload) problems are resolved in the standard makes
little sense to me, as it makes std::filesystem::path mostly unusable (without
extremely ugly work-arounds) in the meantime (which could easily be multiple
years) in code portable between different implementations.

libstdc++ needs to get fixed as soon as possible. It is currently blocking
progress in portable downstream users.


After having said all this, I am not even sure anymore that this bug report
here actually is a duplicate of my bug 95642. I am concerned about implicit
conversion to std::filesystem::path not working, while this bug report is
concerned about std::string_view not working. Please consider re-opening bug
95642.

[Bug libstdc++/90704] [LWG 3430] filesystem::path overloads for file streams are not conforming

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90704

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jörn Heusipp from comment #5)
> > Status: SUSPENDED
> 
> Well, coming from bug 95642, which has been marked as a duplicate of this
> bug, this is *very* disappointing.
> 
> 
> The C++17 standard absolutely clearly specifies that the constructor is
> overloaded for std::filesystem::path. libstdc++'s SFINAE implementation is
> incompatible because it fails for user-defined types that are implicitly
> convertible to std::filesystem::path.

And also for string_view, because that's convertible to filesystem::path, and
that is the topic of the issue reported against the standard.

> In order to at all be able to sensibly use libstdc++ std::filesystem::path
> in my codebase, I would have to put this awful work-around (making tons of
> assumptions about libstdc++'s enable_if expression) into my own type
> mpt::PathString:

Or you can just convert it to filesystem::path explicitly before passing it to
fstream members.


> and just pray and hope that libstdc++ will not change its non-conforming
> SFINAE-using implementation details to some other non-conforming
> implementation details in some future version.

Or just convert to filesystem::path explicitly.

> Can you guarantee, that you wont ever change your non-conforming SFINAE
> implementation whatsoever at all? If not, please just fix it. The standard
> is absolutely clear here.

The standard is flawed and no longer follows its own design. I want to get some
clarity about the intended design.

> After having said all this, I am not even sure anymore that this bug report
> here actually is a duplicate of my bug 95642. I am concerned about implicit
> conversion to std::filesystem::path not working, while this bug report is
> concerned about std::string_view not working. Please consider re-opening bug
> 95642.

They're exactly the same.

string_view is expected to work by converting to filesystem::path, which is the
same as your type.

[Bug libstdc++/90704] [LWG 3430] filesystem::path overloads for file streams are not conforming

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90704

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
btw in practice the fact this is SUSPENDED today is absolutely no different to
the status yesterday. It didn't work in GCC yesterday and it doesn't work
today. The only change is I've publicly stated my intention to discuss this
with the committee before doing anything here.

I could revert the status, and still do nothing here, but that serves no
purpose.

[Bug libstdc++/90704] [LWG 3430] filesystem::path overloads for file streams are not conforming

[manx-bugzilla at problemloesungsmaschine dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90704

--- Comment #8 from Jörn Heusipp <manx-bugzilla at problemloesungsmaschine dot de> ---
(In reply to Jonathan Wakely from comment #6)
> (In reply to Jörn Heusipp from comment #5)

> > and just pray and hope that libstdc++ will not change its non-conforming
> > SFINAE-using implementation details to some other non-conforming
> > implementation details in some future version.
> 
> Or just convert to filesystem::path explicitly.

I'd rather not use std::filesystem whatsoever at all than polluting the whole
code base with redundant explicit conversions. This is exactly what I am trying
to avoid here in the first place by implementing implicit conversion (which the
standard allows me to do). Your implementation breaks this, because you decided
to disregard the standard in order to break string_view conversions. If you
want to break string_view, break it. But please do so selectively and do not
break other implicit conversions at the same time. There is no reason to. This
is just sad.

> > Can you guarantee, that you wont ever change your non-conforming SFINAE
> > implementation whatsoever at all? If not, please just fix it. The standard
> > is absolutely clear here.
> 
> The standard is flawed and no longer follows its own design. I want to get
> some clarity about the intended design.

The intended design of std::basic_fstream::basic_fstream(const
filesystem::path& s, ios_base::openmode mode = ios_base::in | ios_base::out);
(literally from the standard) is unambiguously clear to me. I am allowed to
provide types which are implicitly convertible (as long as no other implicit
conversion causes ambiguity with other overloads). Again, this is IMO unrelated
to whatever the intention for std::string_view might or might not have been.
The fact that they rely on the same overload is coincidence. Are you arguing
that no user-defined type should ever be implicitly convertible to
std::filesystem::path? If so, can you provide reasoning?

[Bug libstdc++/90704] [LWG 3430] filesystem::path overloads for file streams are not conforming

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90704

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|11.0                        |11.2

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 11.1 has been released, retargeting bugs to GCC 11.2.

[Bug libstdc++/90704] [LWG 3430] filesystem::path overloads for file streams are not conforming

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90704

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
             Status|SUSPENDED                   |RESOLVED

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
WG21 has just confirmed that the GCC behaviour is the desired one. The changes
in https://cplusplus.github.io/LWG/issue3430 are in the C++ working draft.