From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb1-xb32.google.com (mail-yb1-xb32.google.com [IPv6:2607:f8b0:4864:20::b32]) by sourceware.org (Postfix) with ESMTPS id B0D133857340 for ; Mon, 11 Apr 2022 15:52:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B0D133857340 Received: by mail-yb1-xb32.google.com with SMTP id w28so6143432ybi.6 for ; Mon, 11 Apr 2022 08:52:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=30KfeaS8c3VMPmmXmbOfWTMwulCQTJGpb5eZYlIxj8M=; b=CpIoOSY+EQ9dFWxWx2zF5JLcQISX1fVJqiuLtLycM70PXRUL5H+2WQxs/gF5HcykBA Bs6RSWZmfTM5j+JGHd0BHvq9Um5V+SXM2OShk9j4OEKQQpTbQkS0uHi9fjVXn6wy0w2S bh26wPNQMd5z4NllmlMN4SkZxjvS6LLyZGu1pEpb5D9ZdodaaQDD4VPedIVd76jq3hlP bpd4X7pb38VouWEmCyFZCZ5hKV6M55Pyu6BebmBUddPtco9nmUyGQs4Zl9BVYzESSoI6 wm+daV+apxFwCQkLPdEJ2pBGUWNuCyIlE0PRmzERqVTSl+9ULv6/dZa7lyXIV84YmN/r Y0zQ== X-Gm-Message-State: AOAM531Pr3OGmG9mxyF2yM25jqye5THk+oa36/gxp0O3E6F1S47VhnWP CIGBWtAFDKIdIxWmLjLk6ZPDoI/6P9kcYeH+60FMNPQs X-Google-Smtp-Source: ABdhPJw1HDv36yWthDqrLN6uPe5F8lLYVvs80/7JuCVsSGJAt6BqrT1xpEIUR8DmM8Pfr+z88LGjAY/mI/8iQZYNqUE= X-Received: by 2002:a25:1882:0:b0:63e:7c08:f570 with SMTP id 124-20020a251882000000b0063e7c08f570mr13523571yby.487.1649692339117; Mon, 11 Apr 2022 08:52:19 -0700 (PDT) MIME-Version: 1.0 References: <20220328191254.913833-1-goldstein.w.n@gmail.com> <20220404165646.570909-1-goldstein.w.n@gmail.com> In-Reply-To: From: Noah Goldstein Date: Mon, 11 Apr 2022 10:52:08 -0500 Message-ID: Subject: Re: [PATCH v6] Add .clang-format style file To: "Carlos O'Donell" Cc: GNU C Library Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-8.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 11 Apr 2022 15:52:22 -0000 On Mon, Apr 11, 2022 at 9:18 AM Carlos O'Donell wrote: > > On 4/4/22 12:56, Noah Goldstein via Libc-alpha wrote: > > Went with version >= 11.0 since it covers most of the major features > > and should be pretty universally accessibly. > > This version looks good to me and has addressed the concerns of the previous reviewers. > This kind of file can be evolved over time if we find that it doesn't exactly meet our > needs, but it should produce correctly formatted code in as many cases as possible. > > LGTM. > > Reviewed-by: Carlos O'Donell Thanks pushed. > > > There are some issues: > > > > 1. 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. i.e: > > Desired: > > ``` > > #ifndef A > > # define B > > #endif > > ``` > > Options: > > ``` > > #ifndef A > > # define B /* Two spaces instead of one. */ > > #endif > > > > #ifndef C > > #define D /* No spaces. */ > > #endif > > ``` > > 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. > > Agreed. > > > > > 2. concatenation of lines missing semi-colons: > > Throughout glibc there are macros used to setup aliasing that are > > outside of functions and don't end in semi-colons i.e: > > ``` > > libc_hidden_def (__pthread_self) > > weak_alias (__pthread_self, pthread_self) > > ``` > > > > clang-format reformats lines like these to: > > ``` > > libc_hidden_def (__pthread_self) weak_alias (__pthread_self, pthread_self) > > ``` > > > > which is generally undesirable. > > It is undesirable. > > However, I'm open to having a consensus discussion if we should just put a ; after these macros > to avoid formatter issues. Please feel free to start that discussion in a new thread. > > We cannot just hide behind established practice and expect all of our tooling to mold itself > to our whims, we need, in some cases to meet tooling half-way. > > > > > Other than those two big concerns there are certainly some questions > > diffs but for the most part it creates a easy to read and consistent > > style. > > Agreed. > > > --- > > .clang-format | 156 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 156 insertions(+) > > create mode 100644 .clang-format > > > > diff --git a/.clang-format b/.clang-format > > new file mode 100644 > > index 0000000000..79530b305a > > --- /dev/null > > +++ b/.clang-format > > @@ -0,0 +1,156 @@ > > +# clang-format file for GLIBC > > +# 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 > > +# . > > +# > > +# Requires clang-format version >= 11.0 > > +# > > +# For more information, see: > > +# > > +# 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. > > +# > > +# To format the current git diff inplace (-i) the follow command can > > +# be used: > > +# $> git diff -U0 --no-color HEAD^ | clang-format-diff -i -p1 > > +# > > +# To just view the diff clang-format would generate: > > +# $> git diff -U0 --no-color HEAD^ | clang-format-diff -p1 > > +# > > +# NB: clang-format-diff, along with other clang-format related tools, > > +# can be found at: /path/to/llvm-project/clang/tools/clang-format/ > > +# > > +# > > +# Based on autogenerated format from: > > +# $> clang-format --style=GNU -dump-config > > +--- > > +AccessModifierOffset: -2 > > +AlignAfterOpenBracket: Align > > +AlignConsecutiveMacros: false > > +AlignConsecutiveAssignments: false > > +AlignConsecutiveBitFields: false > > +AlignConsecutiveDeclarations: false > > +AlignEscapedNewlines: Right > > +AlignOperands: true > > +AlignTrailingComments: true > > +AllowAllArgumentsOnNextLine: true > > +AllowAllParametersOfDeclarationOnNextLine: true > > +AllowShortEnumsOnASingleLine: true > > +AllowShortBlocksOnASingleLine: false > > +AllowShortCaseLabelsOnASingleLine: false > > +AllowShortFunctionsOnASingleLine: All > > +AllowShortLambdasOnASingleLine: All > > +AllowShortIfStatementsOnASingleLine: Never > > +AllowShortLoopsOnASingleLine: false > > +AlwaysBreakAfterDefinitionReturnType: All > > +AlwaysBreakAfterReturnType: AllDefinitions > > +AlwaysBreakBeforeMultilineStrings: false > > +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 > > +BreakStringLiterals: true > > +ColumnLimit: 79 > > +CommentPragmas: '^ IWYU pragma:' > > +CompactNamespaces: false > > +ContinuationIndentWidth: 4 > > +Cpp11BracedListStyle: false > > +DeriveLineEnding: true > > +DerivePointerAlignment: false > > +DisableFormat: false > > +ExperimentalAutoDetectBinPacking: false > > +FixNamespaceComments: false > > +IncludeBlocks: Preserve > > +IncludeCategories: > > + - 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 > > +PenaltyExcessCharacter: 1000000 > > +PenaltyReturnTypeOnItsOwnLine: 60 > > +PointerAlignment: Right > > +ReflowComments: true > > +SortIncludes: false > > +SortUsingDeclarations: true > > +SpaceAfterCStyleCast: true > > +SpaceAfterLogicalNot: false > > +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 > > +TabWidth: 8 > > +UseTab: Always > > +ForEachMacros: > > + - 'FOR_EACH_IMPL' > > + - 'list_for_each' > > + - 'list_for_each_prev' > > + - 'list_for_each_prev_safe' > > +... > > > > -- > Cheers, > Carlos. >