One: [clang-format] About upgrading version

[wateret]

I just faced an actual issue with the old clang-format 3.9 (that we are currently using).

There is this code.

    auto u_handle = std::unique_ptr<void, dlhandle_destroy_t>{
         handle,
         [ id = backend, filename = backend_so ](void *h){if (dlclose(h) == 0){
             VERBOSE(BackendManager) << "Successfully unloaded '" << id << "'(" << filename << ")\n";
   }
   else
   {
     VERBOSE(BackendManager) << "Failed to unload backend '" << id << "'- " << dlerror() << "\n";
   }
 }
 };
 _handle_map.emplace(backend, std::move(u_handle));
 }

Which must have been something like:

    auto u_handle = std::unique_ptr<void, dlhandle_destroy_t>{
         handle, [id = backend, filename = backend_so](void *h) {
           if (dlclose(h) == 0)
           {
             VERBOSE(BackendManager) << "Successfully unloaded '" << id << "'(" << filename << ")\n";
           }
           else
           {
             VERBOSE(BackendManager)
                 << "Failed to unload backend '" << id << "'- " << dlerror() << "\n";
           }
         }};
     _handle_map.emplace(backend, std::move(u_handle));
   }

It seems like 3.9 does not understand "capture with initializer" which is enabled since C++14. For details please see (3) in https://en.cppreference.com/w/cpp/language/lambda#Lambda_capture .

Should we consider upgrading clang-format? AFAIR it had been discussed before, but the conclusion was not upgrading. What about now? We have real issues as we adopt more modern C++ versions.

[wateret]

cc: @Samsung/nnfw @Samsung/nncc

[glistening]

@wateret

1. 
   We may upgrade clang-format for more modern c++ feature syntax.
   We did not upgraded clang-format in previous discussion because it leads to huge changes

It is better time to upgrade clang-format since Ubuntu 16.04 is getting out of life.
If we need more and more modern c++ syntax, we may move recent version of clang-format.

２.
I am not familiar with "capture with initializer". However, your example seems to pass two variables into lambda body. Then, what is the difference passing those directly (w/o initializer)? ­— clang-format-3.9 works for the code below.

auto u_handle = std::unique_ptr<void, dlhandle_destroy_t>{
    handle, [backend, backend_so](void *h) {
      if (dlclose(h) == 0)
        VERBOSE(BackendManager) << "Successfully unloaded '" << backend << "'(" << backend_so
                                << ")\n";
      else
        VERBOSE(BackendManager) << "Failed to unload backend '" << backend << "'- " << dlerror()
                                << "\n";
    }};

[hseok-oh]

5138 is formatting result byclang-format-8 (we can install clang-format-8 by apt-get on ubuntu 16.04, 18.04, and 20.04)

[glistening]

@hseok-oh

What about using more recent version of clang-format than 8? To avoid doing same things when we meet same problem by more modern c++ features in near future.

[hseok-oh]

@glistening If we ignore ubuntu 16.04, we may use clang-format-10 (default on ubuntu 20.04, can install on 18.04).

[glistening]

@hseok-oh Thank you for exact version information that I needed. From #5138, the changed files are smaller than I expected. I concerned more changes eventually would cause several conflicts in PRs in reviews. But it is not. Thus, I am okay for upgrading by two steps or by one time. The former would be more graceful though it requires your devotional effort including (upgrading CI slaves, dockerfile, ...).

[wateret]

auto u_handle = std::unique_ptr<void, dlhandle_destroy_t>{
    handle, [backend, backend_so](void *h) {
      if (dlclose(h) == 0)
        VERBOSE(BackendManager) << "Successfully unloaded '" << backend << "'(" << backend_so
                                << ")\n";
      else
        VERBOSE(BackendManager) << "Failed to unload backend '" << backend << "'- " << dlerror()
                                << "\n";
    }};

@glistening Good point, That might work too. I was just not 100% sure about it would copy backend and backend_so(which are references) so I chose to explicitly copy them with better names.

Anyways this issue is for general cases for example, [a = x+1, r = std::move(q)](...) { ... }.

[wateret]

AFAIR onert(nnfw) and compiler(nncc) both uses the same clang-format, is it correct? @seanshpark @llFreetimell @mhs4670go

[mhs4670go]

@wateret Yes. We use same driver, i.e., nnas.

[hseok-oh]

FYI. format check with clang-format-10: #5144

[glistening]

+1 for clang-format-8 unless someone asks more recent clang-format. Perhaps it is likely to upgrade once more when we use c++17. However, we cannot abandon ubuntu 16.04 right now.

[hseok-oh]

When I make draft PRs, I found out some indent-width is 4, not 2.

https://github.com/Samsung/ONE/blob/46eedd09d380f38670e21e3b5511c580dc433ba7/.clang-format#L43-L44

If above two options are changed to 2, example may become

```diff // "by-copy capture with an initializer"
auto u_handle = std::unique_ptr{

• handle, id = backend, filename = backend_so {
• if (dlclose(h) == 0)
• {
• VERBOSE(BackendManager) << "Successfully unloaded '" << id << "'(" << filename << ")\n";
• }
• else
• {
• VERBOSE(BackendManager)
• << "Failed to unload backend '" << id << "'- " << dlerror() << "\n";
• }
• }};
• handle, id = backend, filename = backend_so {
• if (dlclose(h) == 0)
• {
• VERBOSE(BackendManager) << "Successfully unloaded '" << id << "'(" << filename << ")\n";
• }
• else
• {
• VERBOSE(BackendManager) << "Failed to unload backend '" << id << "'- " << dlerror()
• << "\n";
• }
• }};
  ```

[hseok-oh]

Draft to support independent clang-format-8 migration on each directory: #5223

[wateret]

Thanks to @hseok-oh !