[PATCH v2] Add .clang-format style file
Noah Goldstein
goldstein.w.n@gmail.com
Wed Mar 30 19:30:39 GMT 2022
On Wed, Mar 30, 2022 at 1:17 PM Fangrui Song <maskray@google.com> wrote:
>
> Looks good but I don't know I have the permission to add a Reviewed-by:
> :)
Think it's fine, not like it can break anything. You've certainly
improved the patch.
Unless objection I'll add before I push
>
> To justify this for others, it helps to attach some examples
> how clang-format formatted files match or differ from the existing ones.
>
> Showing some patches also helps as the patch formatter helps users the
> most:
>
> git diff -U0 --no-color 'HEAD^' -- | py3
> path/to/llvm-project/clang/tools/clang-format/clang-format-diff.py -i -p1
Added a comment. If desirable we can add a script for it later.
Here are some diff examples:
Attached are four examples reformatted with clang-format. They
highlight some of the pros/cons of the new automated style.
pthread_rwlock_common.c:
clang-format cleans up excess whitespace before the first comment
[MaxEmptyLinesToKeep: 1] and realigns part of the comments:
-
/* A reader--writer lock that fulfills the POSIX requirements (but operations
on this lock are not necessarily full barriers, as one may interpret the
POSIX requirement about "synchronizing memory"). All critical sections are
@@ -71,18 +70,18 @@
#1 0 0 0 0 Lock is idle (and in a read phase).
#2 0 0 >0 0 Readers have acquired the lock.
#3 0 1 0 0 Lock is not acquired; a writer will try to start a
- write phase.
+ write phase.
It also fixes difficult to ready indentation:
{
rnew = r - (1 << PTHREAD_RWLOCK_READER_SHIFT);
/* If we are the last reader, we also need to unblock any readers
- that are waiting for a writer to go first (PTHREAD_RWLOCK_RWAITING)
- so that they can register while the writer is active. */
+ that are waiting for a writer to go first (PTHREAD_RWLOCK_RWAITING)
+ so that they can register while the writer is active. */
if ((rnew >> PTHREAD_RWLOCK_READER_SHIFT) == 0)
- {
- if ((rnew & PTHREAD_RWLOCK_WRLOCKED) != 0)
- rnew |= PTHREAD_RWLOCK_WRPHASE;
- rnew &= ~(unsigned int) PTHREAD_RWLOCK_RWAITING;
- }
+ {
+ if ((rnew & PTHREAD_RWLOCK_WRLOCKED) != 0)
+ rnew |= PTHREAD_RWLOCK_WRPHASE;
+ rnew &= ~(unsigned int) PTHREAD_RWLOCK_RWAITING;
+ }
aligns conditions nicely:
while ((r & PTHREAD_RWLOCK_WRPHASE) == 0
- && (r & PTHREAD_RWLOCK_WRLOCKED) != 0
- && (r >> PTHREAD_RWLOCK_READER_SHIFT) > 0)
+ && (r & PTHREAD_RWLOCK_WRLOCKED) != 0
+ && (r >> PTHREAD_RWLOCK_READER_SHIFT) > 0)
and cleans up some obvious style mistakes
- done:
+done:
pthread_self.c:
This diff highlights a drawback:
-libc_hidden_def (__pthread_self)
-weak_alias (__pthread_self, pthread_self)
+libc_hidden_def (__pthread_self) weak_alias (__pthread_self, pthread_self)
clang-format doesn't do so well with macros and without a semi-colon
breaking the line will concatenate the two lines which is undesirable
in this case (and the many other equivilent cases in glibc).
pthread_yield.c:
-#if OTHER_SHLIB_COMPAT (libpthread, GLIBC_2_2, GLIBC_2_34)
+#if OTHER_SHLIB_COMPAT(libpthread, GLIBC_2_2, GLIBC_2_34)
This is another issue in clang-format. Inside in #ifdef the
[SpaceBeforeParens: Always] configuration fails.
bench_memcpy.c:
Finally there is the issue of [IndentWidth: 2] applying to macros as
well as code
#ifndef MEMCPY_RESULT
-# define MEMCPY_RESULT(dst, len) dst
-# define MIN_PAGE_SIZE 131072
-# define TEST_MAIN
-# define TEST_NAME "memcpy"
-# include "bench-string.h"
+# define MEMCPY_RESULT(dst, len) dst
+# define MIN_PAGE_SIZE 131072
+# define TEST_MAIN
+# define TEST_NAME "memcpy"
+# include "bench-string.h"
There are active issues with clang-format to allow a seperate
IndentWidth for PP directives
(https://github.com/llvm/llvm-project/issues/42264) but until that is
fixed and we upgrade to a new version we are stuck either not
indenting or over-indenting.
>
> On 2022-03-30, Noah Goldstein via Libc-alpha wrote:
> >Went with version >= 9.0 since it covers most of the major features
> >and should be pretty universally accessibly.
>
> 9.0 => 11.0
Fixed in v2:
>
> >There are some issues, particularly indention of preprocessor
> >directives. Unfortunately there doesn't appear to be a switch for
> >a seperate 'IndentWidth' for preprocessor directives vs. normal
> >code so we are stuck either not indenting the directives or
> >over-indenting them. Chose to over-indent as it generally seems easier
> >to script halving all pre-processor indentations than counting the
> >nested depth and indenting from scratch.
> >
> >Other than the pre-processor directives most of the changes it makes
> >are cleaning up inconsistencies that already existed.
> >---
> > .clang-format | 163 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 163 insertions(+)
> > create mode 100644 .clang-format
> >
> >diff --git a/.clang-format b/.clang-format
> >new file mode 100644
> >index 0000000000..0a8663a596
> >--- /dev/null
> >+++ b/.clang-format
> >@@ -0,0 +1,163 @@
> >+# clang-format file for GLIBC
>
> The first line seems unneeded. The filename self tells.
>
> >+# Copyright (C) 2022 Free Software Foundation, Inc.
> >+# This file is part of the GNU C Library.
> >+#
> >+# The GNU C Library is free software; you can redistribute it and/or
> >+# modify it under the terms of the GNU Lesser General Public
> >+# License as published by the Free Software Foundation; either
> >+# version 2.1 of the License, or (at your option) any later version.
> >+#
> >+# The GNU C Library is distributed in the hope that it will be useful,
> >+# but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> >+# Lesser General Public License for more details.
> >+#
> >+# You should have received a copy of the GNU Lesser General Public
> >+# License along with the GNU C Library; if not, see
> >+# <https://www.gnu.org/licenses/>.
> >+#
> >+# Requires clang-format version >= 11.0
> >+#
> >+# For more information, see:
> >+#
> >+# Documentation/process/clang-format.rst
> >+# https://clang.llvm.org/docs/ClangFormat.html
> >+# https://clang.llvm.org/docs/ClangFormatStyleOptions.html
> >+#
> >+# There are some known cases that this doesn't produce the desired
> >+# style (i.e Preprocessor Directives are over-indented and not
> >+# auto-commented). As a result, this is meant to be a utility to make
> >+# formatting easier, not a definitive standard.
> >+---
> >+# BasedOnStyle: GNU
> >+AccessModifierOffset: -2
> >+AlignAfterOpenBracket: Align
> >+AlignConsecutiveMacros: false
> >+AlignConsecutiveAssignments: false
> >+AlignConsecutiveBitFields: false
> >+AlignConsecutiveDeclarations: false
> >+AlignEscapedNewlines: Right
> >+AlignOperands: true
> >+AlignTrailingComments: true
> >+AllowAllArgumentsOnNextLine: true
> >+AllowAllConstructorInitializersOnNextLine: true
> >+AllowAllParametersOfDeclarationOnNextLine: true
> >+AllowShortEnumsOnASingleLine: true
> >+AllowShortBlocksOnASingleLine: false
> >+AllowShortCaseLabelsOnASingleLine: false
> >+AllowShortFunctionsOnASingleLine: All
> >+AllowShortLambdasOnASingleLine: All
> >+AllowShortIfStatementsOnASingleLine: Never
> >+AllowShortLoopsOnASingleLine: false
> >+AlwaysBreakAfterDefinitionReturnType: All
> >+AlwaysBreakAfterReturnType: AllDefinitions
> >+AlwaysBreakBeforeMultilineStrings: false
> >+AlwaysBreakTemplateDeclarations: MultiLine
> >+BinPackArguments: true
> >+BinPackParameters: true
> >+BraceWrapping:
> >+ AfterCaseLabel: true
> >+ AfterClass: true
> >+ AfterControlStatement: true
> >+ AfterEnum: true
> >+ AfterFunction: true
> >+ AfterNamespace: true
> >+ AfterStruct: true
> >+ AfterUnion: true
> >+ AfterExternBlock: true
> >+ BeforeCatch: true
> >+ BeforeElse: true
> >+ BeforeWhile: true
> >+ IndentBraces: true
> >+ SplitEmptyFunction: true
> >+ SplitEmptyRecord: true
> >+ SplitEmptyNamespace: true
> >+BreakBeforeBinaryOperators: All
> >+BreakBeforeBraces: GNU
> >+BreakBeforeInheritanceComma: false
> >+BreakInheritanceList: BeforeColon
> >+BreakBeforeTernaryOperators: true
> >+BreakConstructorInitializersBeforeComma: false
> >+BreakConstructorInitializers: BeforeColon
> >+BreakStringLiterals: true
> >+ColumnLimit: 79
> >+CommentPragmas: '^ IWYU pragma:'
> >+CompactNamespaces: false
> >+ConstructorInitializerAllOnOneLineOrOnePerLine: false
> >+ConstructorInitializerIndentWidth: 4
> >+ContinuationIndentWidth: 4
> >+Cpp11BracedListStyle: false
> >+DeriveLineEnding: true
> >+DerivePointerAlignment: false
> >+DisableFormat: false
> >+ExperimentalAutoDetectBinPacking: false
> >+FixNamespaceComments: false
> >+ForEachMacros:
> >+ - foreach
> >+ - Q_FOREACH
> >+ - BOOST_FOREACH
> >+IncludeBlocks: Preserve
> >+IncludeCategories:
> >+ - Regex: '^"(llvm|llvm-c|clang|clang-c)/'
> >+ Priority: 2
> >+ - Regex: '^(<|"(gtest|gmock|isl|json)/)'
> >+ Priority: 3
> >+ - Regex: '.*'
> >+ Priority: 1
> >+IncludeIsMainRegex: '(Test)?$'
> >+IndentCaseLabels: false
> >+IndentCaseBlocks: false
> >+IndentGotoLabels: true
> >+IndentWidth: 2
> >+IndentPPDirectives: AfterHash
> >+IndentExternBlock: AfterExternBlock
> >+IndentWrappedFunctionNames: false
> >+InsertTrailingCommas: None
> >+KeepEmptyLinesAtTheStartOfBlocks: true
> >+MacroBlockBegin: ''
> >+MacroBlockEnd: ''
> >+MaxEmptyLinesToKeep: 1
> >+NamespaceIndentation: None
> >+PenaltyBreakAssignment: 2
> >+PenaltyBreakBeforeFirstCallParameter: 19
> >+PenaltyBreakComment: 300
> >+PenaltyBreakFirstLessLess: 120
> >+PenaltyBreakString: 1000
> >+PenaltyBreakTemplateDeclaration: 10
> >+PenaltyExcessCharacter: 1000000
> >+PenaltyReturnTypeOnItsOwnLine: 60
> >+PointerAlignment: Right
> >+ReflowComments: true
> >+SortIncludes: false
> >+SortUsingDeclarations: true
> >+SpaceAfterCStyleCast: true
> >+SpaceAfterLogicalNot: false
> >+SpaceAfterTemplateKeyword: true
> >+SpaceBeforeAssignmentOperators: true
> >+SpaceBeforeCpp11BracedList: false
> >+SpaceBeforeCtorInitializerColon: true
> >+SpaceBeforeInheritanceColon: true
> >+SpaceBeforeParens: Always
> >+SpaceBeforeRangeBasedForLoopColon: true
> >+SpaceInEmptyBlock: false
> >+SpaceInEmptyParentheses: false
> >+SpacesBeforeTrailingComments: 1
> >+SpacesInAngles: false
> >+SpacesInConditionalStatement: false
> >+SpacesInContainerLiterals: true
> >+SpacesInCStyleCastParentheses: false
> >+SpacesInParentheses: false
> >+SpacesInSquareBrackets: false
> >+SpaceBeforeSquareBrackets: false
> >+Standard: Cpp03
> >+StatementMacros:
> >+ - Q_UNUSED
> >+ - QT_REQUIRE_VERSION
> >+TabWidth: 8
> >+UseTab: Never
> >+ForEachMacros:
> >+ - 'FOR_EACH_IMPL'
> >+ - 'list_for_each'
> >+ - 'list_for_each_prev'
> >+ - 'list_for_each_prev_safe'
> >+...
> >--
> >2.25.1
> >
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v1-0004-clang-format-bench-memcpy.c.patch
Type: text/x-patch
Size: 1304 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/libc-alpha/attachments/20220330/85aa900f/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v1-0002-clang-format-pthread_self.c.patch
Type: text/x-patch
Size: 680 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/libc-alpha/attachments/20220330/85aa900f/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v1-0003-clang-format-pthread_yield.c.patch
Type: text/x-patch
Size: 698 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/libc-alpha/attachments/20220330/85aa900f/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v1-0001-clang-format-pthread_rwlock_common.c.patch
Type: text/x-patch
Size: 55762 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/libc-alpha/attachments/20220330/85aa900f/attachment-0007.bin>
More information about the Libc-alpha
mailing list