Skip to content

[libunwind] Fix an inconsistent indentation (NFC) #72314

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 15, 2023

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented Nov 14, 2023

A fix was tried in
d080b5f but the line is still inconsistent with the surrounding code.

A fix was tried in
llvm@d080b5f
but the line is still inconsistent with the surrounding code.
@aheejin aheejin requested a review from mstorsjo November 14, 2023 21:44
@aheejin aheejin requested a review from a team as a code owner November 14, 2023 21:44
@llvmbot
Copy link
Member

llvmbot commented Nov 14, 2023

@llvm/pr-subscribers-libunwind

Author: Heejin Ahn (aheejin)

Changes

A fix was tried in
d080b5f but the line is still inconsistent with the surrounding code.


Full diff: https://github.com/llvm/llvm-project/pull/72314.diff

1 Files Affected:

  • (modified) libunwind/include/__libunwind_config.h (+1-1)
diff --git a/libunwind/include/__libunwind_config.h b/libunwind/include/__libunwind_config.h
index 2444e7286b637f1..8db336b2d727ce7 100644
--- a/libunwind/include/__libunwind_config.h
+++ b/libunwind/include/__libunwind_config.h
@@ -199,7 +199,7 @@
 # define _LIBUNWIND_TARGET_RISCV 1
 # define _LIBUNWIND_TARGET_VE 1
 # define _LIBUNWIND_TARGET_S390X 1
- #define _LIBUNWIND_TARGET_LOONGARCH 1
+# define _LIBUNWIND_TARGET_LOONGARCH 1
 # define _LIBUNWIND_CONTEXT_SIZE 167
 # define _LIBUNWIND_CURSOR_SIZE 204
 # define _LIBUNWIND_HIGHEST_DWARF_REGISTER 287

Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff a40900211a9c187a69b0c3028056275387ce7f1a 6fdc1096db89073020ffb9daa248d0c18c8992ee -- libunwind/include/__libunwind_config.h
View the diff from clang-format here.
diff --git a/libunwind/include/__libunwind_config.h b/libunwind/include/__libunwind_config.h
index 8db336b2d7..c5ee5fee8d 100644
--- a/libunwind/include/__libunwind_config.h
+++ b/libunwind/include/__libunwind_config.h
@@ -199,7 +199,7 @@
 # define _LIBUNWIND_TARGET_RISCV 1
 # define _LIBUNWIND_TARGET_VE 1
 # define _LIBUNWIND_TARGET_S390X 1
-# define _LIBUNWIND_TARGET_LOONGARCH 1
+#define _LIBUNWIND_TARGET_LOONGARCH 1
 # define _LIBUNWIND_CONTEXT_SIZE 167
 # define _LIBUNWIND_CURSOR_SIZE 204
 # define _LIBUNWIND_HIGHEST_DWARF_REGISTER 287

Copy link
Member

@mstorsjo mstorsjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! (I have no idea how I botched that previous fix commit...)

Copy link
Member

@arichardson arichardson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making it consistent is good, but it sounds like we should update the .clang-format config file or reformat this whole file to not be indented.

@aheejin
Copy link
Member Author

aheejin commented Nov 14, 2023

Making it consistent is good, but it sounds like we should update the .clang-format config file or reformat this whole file to not be indented.

I'm not sure what the clang-format policy for libunwind is. How about landing this as is and updating .clang-format or excluding this file as a follow-up, in a way you and other libunwind maintainers think is appropriate?

@arichardson
Copy link
Member

arichardson commented Nov 15, 2023

Making it consistent is good, but it sounds like we should update the .clang-format config file or reformat this whole file to not be indented.

I'm not sure what the clang-format policy for libunwind is. How about landing this as is and updating .clang-format or excluding this file as a follow-up, in a way you and other libunwind maintainers think is appropriate?

Sorry if that wasn't clear, I think your change should be merged. I just noticed that the current style does not match the configured one. This should eventually be fixed as a follow up change.

@ldionne
Copy link
Member

ldionne commented Nov 15, 2023

LGTM, the AIX failure seems unrelated.

@ldionne ldionne merged commit cb1a639 into llvm:main Nov 15, 2023
@aheejin aheejin deleted the libunwind_indent branch November 15, 2023 18:31
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants