[libc++] add `inline` for __open in ifstream and ofstream

[jasonliu]

Summary

When building with gcc on AIX, it seems that gcc does not like the always_inline without the inline keyword.
Such error is observed:

/home/jasonli/llvm/build2/include/c++/v1/fstream: In function 'bool std::__1::__fs::filesystem::detail::{anonymous}::copy_file_impl(std::__1::__fs::filesystem::detail::{anonymous}::FileDescriptor&, std::__1::__fs::filesystem::detail::{anonymous}::FileDescriptor&, std::__1::error_code&)':
/home/jasonli/llvm/build2/include/c++/v1/fstream:1329:6: error: inlining failed in call to always_inline 'void std::__1::basic_ifstream<_CharT, _Traits>::__open(int, std::__1::ios_base::openmode) [with _CharT = char; _Traits = std::__1::char_traits<char>]': function body not available
/home/jasonli/GitHub/llvm-project/libcxx/src/filesystem/operations.cpp:856:14: note: called from here
     in.__open(read_fd.fd, ios::binary);
     ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~

So adding the inline keywords in for __open in ifstream and ofstream. That will also make it consistent with __open in basic_filebuf (it seems we added inline there for gcc build as well).

[• Quuxplusone]

libcxx/include/fstream

1546

I think you should just copy what close does on lines 1194 and 1555. Namely, put _LIBCPP_INLINE_VISIBILITY (only) on the declaration and inline (only) on the definition. Does that make GCC happy enough?

[jasonliu]

libcxx/include/fstream

1546

Thanks for the speedy review. Yes, it does. Do we want to make the matching change for basic_filebuf's __open as well? Currently it's using the same style as what we have here.

[• Quuxplusone]

libcxx/include/fstream

1546

My impression from the code is that we are really trying to have (only?) two different kinds of member functions:

• ones like basic_filebuf::open(const char*, ios_base::openmode) and basic_filebuf::close and basic_filebuf::swap, which are marked neither inline nor _LIBCPP_INLINE_VISIBILITY;
• ones like basic_filebuf::open(const string&, ios_base::openmode) and basic_filebuf::is_open, which are marked inline on the definition and _LIBCPP_INLINE_VISIBILITY on the declaration.

basic_filebuf::__open is currently not in either of these categories. I think we should change it to one or the other. But beware that I don't understand the criteria by which we choose one or the other; in particular I don't know if there might be ABI implications somehow?

[jasonliu]

Making basic_filebuf's __open consistent with other __open.

[jasonliu]

libcxx/include/fstream

1546

I think it makes sense to to choose the first categories you listed there, as we already have _LIBCPP_INLINE_VISIBILITY on those declarations.

[jasonliu]

@ldionne Hi Louis, what do you think about this patch? Do you have any ABI implication concern?

libcxx/include/fstream

#endif
     basic_filebuf* open(const string& __s, ios_base::openmode __mode);
 
 #if _LIBCPP_STD_VER >= 17 && !defined(_LIBCPP_HAS_NO_FILESYSTEM_LIBRARY)
     _LIBCPP_AVAILABILITY_FILESYSTEM _LIBCPP_INLINE_VISIBILITY
     basic_filebuf* open(const _VSTD_FS::path& __p, ios_base::openmode __mode) {
       return open(__p.c_str(), __mode);
     }
 #endif
-    inline _LIBCPP_INLINE_VISIBILITY
+    _LIBCPP_INLINE_VISIBILITY
     basic_filebuf* __open(int __fd, ios_base::openmode __mode);
 #endif
     basic_filebuf* close();
 
     _LIBCPP_INLINE_VISIBILITY
     inline static const char*
     __make_mdstring(ios_base::openmode __mode) _NOEXCEPT;
 
if (__file_ == nullptr)
         } else
           __rt = nullptr;
       }
     }
     return __rt;
 }
 
 template <class _CharT, class _Traits>
-inline _LIBCPP_INLINE_VISIBILITY
+inline
 basic_filebuf<_CharT, _Traits>*
 basic_filebuf<_CharT, _Traits>::__open(int __fd, ios_base::openmode __mode) {
   basic_filebuf<_CharT, _Traits>* __rt = nullptr;
   if (__file_ == nullptr) {
     if (const char* __mdstr = __make_mdstring(__mode)) {
       __rt = this;
       __file_ = fdopen(__fd, __mdstr);
       if (__file_) {

 {
     if (__sb_.open(__s, __mode | ios_base::in))
         this->clear();
     else
         this->setstate(ios_base::failbit);
 }
 
 template <class _CharT, class _Traits>
+inline
 void basic_ifstream<_CharT, _Traits>::__open(int __fd,
                                              ios_base::openmode __mode) {
   if (__sb_.__open(__fd, __mode | ios_base::in))
     this->clear();
   else
     this->setstate(ios_base::failbit);
 }
 #endif

 {
     if (__sb_.open(__s, __mode | ios_base::out))
         this->clear();
     else
         this->setstate(ios_base::failbit);
 }
 
 template <class _CharT, class _Traits>
+inline
 void basic_ofstream<_CharT, _Traits>::__open(int __fd,
                                              ios_base::openmode __mode) {
   if (__sb_.__open(__fd, __mode | ios_base::out))
     this->clear();
   else
     this->setstate(ios_base::failbit);
 }
 #endif