[libcxx-commits] [PATCH] D116234: [libc++][NFC] Reformat <__filesystem/operations.h>

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone requested changes to this revision.
Quuxplusone added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/__filesystem/operations.h:33-59
+_LIBCPP_FUNC_VIS path __absolute(const path&, error_code* __ec = nullptr);
+_LIBCPP_FUNC_VIS path __canonical(const path&, error_code* __ec = nullptr);
+_LIBCPP_FUNC_VIS bool __copy_file(const path& __from, const path& __to, copy_options __opt, error_code* __ec = nullptr);
+_LIBCPP_FUNC_VIS void __copy_symlink(const path& __existing_symlink, const path& __new_symlink, error_code* __ec = nullptr);
+_LIBCPP_FUNC_VIS void __copy(const path& __from, const path& __to, copy_options __opt, error_code* __ec = nullptr);
+_LIBCPP_FUNC_VIS bool __create_directories(const path& p, error_code* ec = nullptr);
+_LIBCPP_FUNC_VIS void __create_directory_symlink(const path& __to, const path& __new_symlink, error_code* __ec = nullptr);
----------------
FWIW, these 26 lines seem uncontroversial to me. Below this point, stuff gets messy.


================
Comment at: libcxx/include/__filesystem/operations.h:61
+
+inline _LIBCPP_INLINE_VISIBILITY path absolute(const path& __p)                    { return __absolute(__p); }
+inline _LIBCPP_INLINE_VISIBILITY path absolute(const path& __p, error_code& __ec)  { return __absolute(__p, &__ec); }
----------------
`s/_LIBCPP_INLINE_VISIBILITY/_LIBCPP_HIDE_FROM_ABI/` throughout this file — it'll save you some column width. Perhaps do that as a separate NFC commit (even though the whole thing is NFC anyway).


================
Comment at: libcxx/include/__filesystem/operations.h:371-372
 
-inline _LIBCPP_INLINE_VISIBILITY path temp_directory_path() {
-  return __temp_directory_path();
-}
-
-inline _LIBCPP_INLINE_VISIBILITY path temp_directory_path(error_code& __ec) {
-  return __temp_directory_path(&__ec);
-}
+inline _LIBCPP_INLINE_VISIBILITY path temp_directory_path()                 { return __temp_directory_path(); }
+inline _LIBCPP_INLINE_VISIBILITY path temp_directory_path(error_code& __ec) { return __temp_directory_path(&__ec); }
 
----------------
For all of these `inline` one-liners, I'd strongly prefer that they be formatted consistently. I don't have a strong preference for
```
inline _LIBCPP_HIDE_FROM_ABI function header() { return oneliner; }
```
versus
```
inline _LIBCPP_HIDE_FROM_ABI
function header() { return oneliner; }
```
versus
```
inline _LIBCPP_HIDE_FROM_ABI function header()
  { return oneliner; }
```
versus
```
inline _LIBCPP_HIDE_FROM_ABI function header() {
  return oneliner;
}
```
versus
```
inline _LIBCPP_HIDE_FROM_ABI
function header() {
  return oneliner;
}
```
but let's be consistent. Also, if we choose anything other than the first option (all-on-one-source-line), then let's add one blank line after each function, so they don't all run together visually.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116234/new/

https://reviews.llvm.org/D116234