WIP: Use std filesystem path

[blowekamp]

Add std::filesystem::path to ImageIOBase for eventual wchar support for paths.

This proposes an approach to remove direct access to m_FileName, with the eventual end goal to use GetFilePath and "legacy" GetFileName methods.

Add Set/Get FilePath member functions to ImageIOBase, with the future
plan to replace the m_FileName member variable with these access
member functions. The m_FileName is directly access by derived ImageIO
classes.

The m_FileName is now "constant" to disallow derived classes from
directly writing to the variable.  The SetFilePath method updates the
legacy const m_FileName to maintain compatibility.

[N-Dekker]

I'm pretty sure that this const_cast<std::string &>(m_FileName) = filepath.string() is undefined behavior, when m_FileName is defined by const std::string m_FileName{}. The compiler is allowed to assume that m_FileName is never modified, when it is declared like that.

You could work around it by declaring m_FileName as a const reference to a non-const string, instead:

const std::string & m_FileName = m_NonConstFileName;

[N-Dekker]

It would be preferable to make m_FileName private, instead of protected. Then the workaround (const_cast<std::string &>) would not be necessary at all. ImageIOBase has a GetFileName and a SetFileName anyway, so direct access to m_FileName should not be necessary.

[blowekamp]

m_FileName is directly access by all ImageIO and it not trivial to make the change to use the GetFileName accessor. The end goal is to use the GetFilePath accessor, so the idea to leave the compatible "m_FileName" and switch to "GetFilePath".

[dzenanz]

This work is quite welcome! But perhaps wait with this potentially disruptive change until 5.4 is released?

[dzenanz]

What is the need for this indirection?

[blowekamp]

I am adding a similar type in SimpleITK to transition from std::string to std::filesystem::path which is being conditioned to the preprocessor (maybe). It also made wrapping with SWIG a little easier.

It is shorter?

[N-Dekker]

Personally I would prefer to just use std::filesystem::path. (Unless it would need to be configurable, as in SimpleITK.) ITK already has so many typedefs (or type aliases, if you like). When doing code analysis, I often lose time, searching for the definition of a typedef. It doesn't always make the code more readable to have all those typedefs.

[N-Dekker]

@blowekamp Very interesting, thanks! I definitely like having a separate type for file paths, rather than just "std::string", for improved type safety. I'm also curious to see if it does improve non-ASCII character support in file names, on Windows.

[seanm]

It looks like adopting std::filesystem would require increasing ITK's minimum macOS to 10.15:

https://stackoverflow.com/questions/58667853/does-use-of-c17-stdfilesystem-require-macos-10-15-xcode-11-1

Even though C++17 compilers exist on older version of macOS, the STL shipped on older versions does not support std::filesystem, from what I gather.

That would suck for me, I still support back to 10.13.

[thewtex]

I still support back to 10.13.

Our Python packages also require 10.11.