[libcxxabi] NFC: fix incorrect indentation of braces

[zhouyizhou]

Summary

Some functions in cxa_exception_storage.cpp have incorrect indentation of braces,
use clang-format to fix them

Signed-off-by: Zhouyi Zhou <[e-mail]>

[• Quuxplusone]

libcxxabi/src/cxa_exception_storage.cpp

102–103

LGTM, FWIW.

It's surprising that clang-format adds } // namespace foo closing comments, but doesn't add } // extern "C" closing comments: this lonely brace is a bit confusing and I think it would benefit from an // extern "C" comment.

[ldionne]

FWIW, I dislike that we don't indent stuff at all inside namespaces -- often I find it useful to indent things inside short-lived namespaces. But let's go for simplicity and consistency.

Did you look into clang-formatting other files in libc++abi? What would the diff look like?

[zhouyizhou]

In D109408#2988417, @ldionne wrote:

FWIW, I dislike that we don't indent stuff at all inside namespaces -- often I find it useful to indent things inside short-lived namespaces. But let's go for simplicity and consistency.

Did you look into clang-formatting other files in libc++abi? What would the diff look like?

Thanks for reviewing,
I will look into other files in libc++abi one by one, and report what I have found ;-)
Thanks again
Zhouyi

libcxxabi/src/cxa_exception_storage.cpp

102–103

Thanks for reviewing,
clang-format do behave a little strange to me. The result of invoking clang-format in git cloned directory is diffent from invoking clang-format in other directory ( for example /tmp).
By the way, I have not write access to LLVM, so I need someone to commit for me.

Thanks again ;-)
Zhouyi

[zhouyizhou]

This comment was removed by zhouyizhou.

[zhouyizhou]

In D109408#2988417, @ldionne wrote:

FWIW, I dislike that we don't indent stuff at all inside namespaces -- often I find it useful to indent things inside short-lived namespaces. But let's go for simplicity and consistency.

Did you look into clang-formatting other files in libc++abi? What would the diff look like?

I take a general view of .cpp(.h) files in libc++abi, they are distributed in several directories. I do an experiment with
fuzz/cxa_demangle_fuzzer.cpp using clang-format, the result is as follows:

• cxa_demangle_fuzzer.cpp.orig 2021-09-09 02:19:17.117872938 +0800

+++ cxa_demangle_fuzzer.cpp 2021-09-09 02:19:34.845879852 +0800
@@ -1,15 +1,15 @@
-#include <stdint.h>
#include <stddef.h>
-#include <string.h>
+#include <stdint.h>
#include <stdlib.h>
-extern "C" char *
-cxa_demangle(const char *mangled_name, char *buf, size_t *n, int *status);
+#include <string.h>
+extern "C" char *cxa_demangle(const char *mangled_name, char *buf, size_t *n,
+ int *status);

extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {

• char *str = new char[size+1];

+ char *str = new char[size + 1];

memcpy(str, data, size);
str[size] = 0;
free(__cxa_demangle(str, 0, 0, 0));

• delete [] str;

+ delete[] str;

return 0;

}

As a beginner of LLVM and C++, I guess clang-formatting is meaningful ;-)

I guess we can clang-format a set of files at a time, and step-by-step clang-format all the .cpp(.h) files in libc++abi and fine-tune the results as how Quuxplusone did it ,
what's your opinion? If it is OK, I am very honored and pleased to do this work ;-)

Zhouyi

[zhouyizhou]

Hi,
I have no write access to LLVM, can you commit D109408 for me?
Thanks a lot
Zhouyi Zhou <[e-mail]>

[• Quuxplusone]

I was going to land this just now, but then realized that I've lost the thread of why we're doing most of this. Why are we touching clang/test/SemaCXX/? So I'm going to land just the fixing of the curly braces in libcxxabi/src/.

libcxxabi/src/cxa_exception_storage.cpp

111

This commented-out line confuses me.

[zhouyizhou]

In D109408#2996092, @Quuxplusone wrote:

I was going to land this just now, but then realized that I've lost the thread of why we're doing most of this. Why are we touching clang/test/SemaCXX/? So I'm going to land just the fixing of the curly braces in libcxxabi/src/.

Hi,
Very sorry to bring you so much trouble.
I am new to LLVM, and I am only intend to fix the curly braces in libcxxabi/src/,
I followed https://www.llvm.org/docs/Phabricator.html#phabricator-reviews to make a patch using:
git diff -U999999 u > /tmp/mypatch.patch
I am also very confused why clang/test/SemaCXX/ appears in mypatch.patch.
Sorry again.
And thanks for landing the fixing of the curly braces in libcxxabi/src/ for me.

Zhouyi

[ldionne]

In D109408#2996128, @zhouyizhou wrote:

In D109408#2996092, @Quuxplusone wrote:

I was going to land this just now, but then realized that I've lost the thread of why we're doing most of this. Why are we touching clang/test/SemaCXX/? So I'm going to land just the fixing of the curly braces in libcxxabi/src/.

Hi,
Very sorry to bring you so much trouble.
I am new to LLVM, and I am only intend to fix the curly braces in libcxxabi/src/,
I followed https://www.llvm.org/docs/Phabricator.html#phabricator-reviews to make a patch using:
git diff -U999999 u > /tmp/mypatch.patch
I am also very confused why clang/test/SemaCXX/ appears in mypatch.patch.
Sorry again.
And thanks for landing the fixing of the curly braces in libcxxabi/src/ for me.

Zhouyi

When you submit patches, please follow https://libcxx.llvm.org/Contributing.html. In particular, you should follow these instructions for submitting using arc diff -- it works a lot better than the web interface: https://www.llvm.org/docs/Phabricator.html#requesting-a-review-via-the-command-line.

Thanks for the patch!

[zhouyizhou]

In D109408#3018830, @ldionne wrote:

In D109408#2996128, @zhouyizhou wrote:

In D109408#2996092, @Quuxplusone wrote:

I was going to land this just now, but then realized that I've lost the thread of why we're doing most of this. Why are we touching clang/test/SemaCXX/? So I'm going to land just the fixing of the curly braces in libcxxabi/src/.

Hi,
Very sorry to bring you so much trouble.
I am new to LLVM, and I am only intend to fix the curly braces in libcxxabi/src/,
I followed https://www.llvm.org/docs/Phabricator.html#phabricator-reviews to make a patch using:
git diff -U999999 u > /tmp/mypatch.patch
I am also very confused why clang/test/SemaCXX/ appears in mypatch.patch.
Sorry again.
And thanks for landing the fixing of the curly braces in libcxxabi/src/ for me.

Zhouyi

When you submit patches, please follow https://libcxx.llvm.org/Contributing.html. In particular, you should follow these instructions for submitting using arc diff -- it works a lot better than the web interface: https://www.llvm.org/docs/Phabricator.html#requesting-a-review-via-the-command-line.

Thanks for the patch!

Thanks for the instructions
I will follow https://libcxx.llvm.org/Contributing.html and will follow the instructions to requesting a review via the command line.
It's a very good opportunity to improve myself ;-)
Thanks again
Zhouyi

libcxxabi/src/cxa_exception_storage.cpp

 
 #if defined(_LIBCXXABI_HAS_NO_THREADS)
 
 namespace __cxxabiv1 {
 extern "C" {
     static __cxa_eh_globals eh_globals;
     __cxa_eh_globals *__cxa_get_globals() { return &eh_globals; }
     __cxa_eh_globals *__cxa_get_globals_fast() { return &eh_globals; }
-    }
-}
+} // extern "C"
+} // namespace __cxxabiv1
 
 #elif defined(HAS_THREAD_LOCAL)
 
 namespace __cxxabiv1 {
-
 namespace {
-    __cxa_eh_globals * __globals () {
+    __cxa_eh_globals *__globals() {
         static thread_local __cxa_eh_globals eh_globals;
         return &eh_globals;
-        }
-    }
+} // namespace
 
 extern "C" {
-    __cxa_eh_globals * __cxa_get_globals      () { return __globals (); }
-    __cxa_eh_globals * __cxa_get_globals_fast () { return __globals (); }
-    }
-}
+    __cxa_eh_globals *__cxa_get_globals() { return __globals(); }
+    __cxa_eh_globals *__cxa_get_globals_fast() { return __globals(); }
+} // extern "C"
+} // namespace __cxxabiv1
 
 #else
 
 #include "abort_message.h"
 #include "fallback_malloc.h"
 
 #if defined(__ELF__) && defined(_LIBCXXABI_LINK_PTHREAD_LIB)
 #pragma comment(lib, "pthread")
 #endif
 
 //  In general, we treat all threading errors as fatal.
 //  We cannot call std::terminate() because that will in turn
 //  call __cxa_get_globals() and cause infinite recursion.
 
 namespace __cxxabiv1 {
 namespace {
     std::__libcpp_tls_key key_;
     std::__libcpp_exec_once_flag flag_ = _LIBCPP_EXEC_ONCE_INITIALIZER;
 
-    void _LIBCPP_TLS_DESTRUCTOR_CC destruct_ (void *p) {
-        __free_with_fallback ( p );
-        if ( 0 != std::__libcpp_tls_set ( key_, NULL ) )
+    void _LIBCPP_TLS_DESTRUCTOR_CC destruct_(void *p) {
+        __free_with_fallback(p);
+        if (0 != std::__libcpp_tls_set(key_, NULL))
             abort_message("cannot zero out thread value for __cxa_get_globals()");
-        }
 
-    void construct_ () {
-        if ( 0 != std::__libcpp_tls_create ( &key_, destruct_ ) )
+    void construct_() {
+        if (0 != std::__libcpp_tls_create(&key_, destruct_))
             abort_message("cannot create thread specific key for __cxa_get_globals()");
-        }
-}
+} // namespace
 
 extern "C" {
-    __cxa_eh_globals * __cxa_get_globals () {
-    //  Try to get the globals for this thread
-        __cxa_eh_globals* retVal = __cxa_get_globals_fast ();
+    __cxa_eh_globals *__cxa_get_globals() {
+        // Try to get the globals for this thread
+        __cxa_eh_globals *retVal = __cxa_get_globals_fast();
 
-    //  If this is the first time we've been asked for these globals, create them
-        if ( NULL == retVal ) {
-            retVal = static_cast<__cxa_eh_globals*>
-                        (__calloc_with_fallback (1, sizeof (__cxa_eh_globals)));
-            if ( NULL == retVal )
+        // If this is the first time we've been asked for these globals, create them
+        if (NULL == retVal) {
+            retVal = static_cast<__cxa_eh_globals*>(
+                __calloc_with_fallback(1, sizeof(__cxa_eh_globals)));
+            if (NULL == retVal)
                 abort_message("cannot allocate __cxa_eh_globals");
-            if ( 0 != std::__libcpp_tls_set ( key_, retVal ) )
+            if (0 != std::__libcpp_tls_set(key_, retVal))
                abort_message("std::__libcpp_tls_set failure in __cxa_get_globals()");
-           }
         return retVal;
-        }
 
     // Note that this implementation will reliably return NULL if not
     // preceded by a call to __cxa_get_globals().  This is an extension
     // to the Itanium ABI and is taken advantage of in several places in
     // libc++abi.
-    __cxa_eh_globals * __cxa_get_globals_fast () {
-    //  First time through, create the key.
+    __cxa_eh_globals *__cxa_get_globals_fast() {
+        // First time through, create the key.
         if (0 != std::__libcpp_execute_once(&flag_, construct_))
             abort_message("execute once failure in __cxa_get_globals_fast()");
-//        static int init = construct_();
         return static_cast<__cxa_eh_globals*>(std::__libcpp_tls_get(key_));
-        }
+} // extern "C"
+} // namespace __cxxabiv1
 
-}
-}
 #endif