fs::path to std::string when constructing file streams
#33545
The code fails to compile using the LLVM MinGW toolchain.
When building with libc++ that has LWG 3430 implemented, constructing std::ifstream or std::ofstream from an fs::path object triggers its implicit conversion to std::string. This behavior has been undesirable ever since fs::path was introduced. See, for example: https://github.com/bitcoin/bitcoin/blob/a33bd767a37dccf39a094d03c2f62ea81633410f/src/util/fs.h#L54-L55
I guess this means any fs::path would have to be explicitly cast to a std::fs::path, which seems fine to do.
This is one way of doing it.
I guess you are asking how to deal with the fs linter?
This, and something like operator std::string() const = delete; should be added to fs::path.
I guess this means any fs::path would have to be explicitly cast to a std::fs::path, which seems fine to do.
This is one way of doing it.
Another approach would be to restore fsbridge::ifstream and fsbridge::ofstream, which were removed in 41d7166c8a598b604aad6c6b1d88ad46e23be247.
What is the bug exactly?
The current description is confusing because it says “constructing std::ifstream or std::ofstream from an fs::path object triggers its implicit conversion to std::string”. But there aren’t any unsafe impilcit conversion from paths to strings, there is only a safe implicit conversion to the native string type.
So what is the bug? Are there compile errors on windows? Is this the concern? Would be good to say what the specific bug or expected behavior is.
I guess this means any fs::path would have to be explicitly cast to a std::fs::path, which seems fine to do.
This is one way of doing it.
I don’t understand this response. What are the other ways of doing it?
I think it might be nice to add a helper method like const std::filesystem_path& std_path() const { return *this; } so explicit conversions do not need to be very verbose. But I’m not sure if this would address the issue being reported.
I guess you are asking how to deal with the fs linter?
This, and something like
operator std::string() const = delete;should be added tofs::path.
What is the issue with the fs linter? What problem would this deleted operator solve? I could imagine it maybe being useful to trigger compile errors on unix if the code might fail to compile on windows? But again if this would address the issue or what exactly the issue is.
What is the bug exactly?
The code fails to compile using the LLVM MinGW toolchain.
So what is the bug? Are there compile errors on windows? Is this the concern? Would be good to say what the specific bug or expected behavior is.
Yes, my understanding is there’d be compile errors, but I had difficulty as well figuring this out. My understanding is that there are no unsafe conversions or other bugs. So my suggestion was to just add an explicit cast. Your std_path suggestion seems fine as well.
I guess this means any fs::path would have to be explicitly cast to a std::fs::path, which seems fine to do.
This is one way of doing it.
I don’t understand this response. What are the other ways of doing it?
I guess this means any fs::path would have to be explicitly cast to a std::fs::path, which seems fine to do.
This is one way of doing it.
Another approach would be to restore
fsbridge::ifstreamandfsbridge::ofstream, which were removed in 41d7166.
What is the bug exactly?
The code fails to compile using the LLVM MinGW toolchain
Thanks this is very helpful. Would recommend adding this to the issue description and probably making it the first sentence. I think deleting operator std::string with comment like // Disallow std::string conversion to ensure code is portable, because this operator is not defined on windows would be a good solution to ensure the problem does not go undetected.
I wouldn’t want fsbridge::ifstream and fsbridge::ofstream restored. Or I don’t see how that would help anything. It seems much nicer to use standard classes.
would be a good solution to ensure the problem does not go undetected.
For reference, the MWE for this would be https://godbolt.org/z/KeKq7dxfW (Our fs namespace with operator std::string deleted and a minimal main function)