From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x632.google.com (mail-pl1-x632.google.com [IPv6:2607:f8b0:4864:20::632]) by sourceware.org (Postfix) with ESMTPS id 5FB76388882E for ; Wed, 30 Mar 2022 20:09:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5FB76388882E Received: by mail-pl1-x632.google.com with SMTP id i11so21540984plr.1 for ; Wed, 30 Mar 2022 13:09: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=UziZdAv86uaWgA/QYd7u9tcDaeCtZsQJDvyjGMYyhOs=; b=J6WvRTzgE8D/0YRyyAZ6/TI5cvH1vk1wG0wTxpD7/MDaQItsvwMZcugb91fIimVUJv 25vsxWgAhnUZHWJaeCv2hbXQSFHvsOEcYhiwa2eoL2T+hQSKjw/UH3fXpLItWdsyzD1q qcUqU8ndoLIQE6h63FjSvpdnyL8ORNbbyxIIT+M7woQFbZ0Rj8oOPKLAYcP5VaQhAgIv 7EfMx0KVKcalxSZgz3jLZOVNVkcMbaBAuSrc+m2E+M2J/Q/5SX+G9BUhXeyTDwuXpMjq 4TzjgF+oqPxHzk4lMaIXPk7LxPcEN2SL6UJTDWqffh9vcubXdBz4I6IKk6nCI3KiEWnB rR0A== X-Gm-Message-State: AOAM533w7K/EkegMFGvECPdp5JZRlZH0Y0oOfOVqKPjXFskDpuT3yCXm 3lpjr4vvQ5Vb8Ixt67JXIb5KdVl14e2y+Fovc7uEN/WX X-Google-Smtp-Source: ABdhPJybj113he0C18XPzrJtiREgH0Gk7eFoYQXxO3sN2+3Xi9ZcasWe5VWYBpt2Gzqj7HfApGbEJMQQZ0Z5DvV4/hk= X-Received: by 2002:a17:90b:4c09:b0:1c7:6c8:43c8 with SMTP id na9-20020a17090b4c0900b001c706c843c8mr1388874pjb.208.1648670958385; Wed, 30 Mar 2022 13:09:18 -0700 (PDT) MIME-Version: 1.0 References: <20220328191254.913833-1-goldstein.w.n@gmail.com> <20220330192714.3177346-1-goldstein.w.n@gmail.com> <87k0cbgqgk.fsf@mid.deneb.enyo.de> In-Reply-To: <87k0cbgqgk.fsf@mid.deneb.enyo.de> From: Noah Goldstein Date: Wed, 30 Mar 2022 15:09:07 -0500 Message-ID: Subject: Re: [PATCH v3] Add .clang-format style file To: Florian Weimer Cc: Noah Goldstein via Libc-alpha Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.3 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: Wed, 30 Mar 2022 20:09:22 -0000 On Wed, Mar 30, 2022 at 3:01 PM Florian Weimer wrote: > > * Noah Goldstein via Libc-alpha: > > > Went with version >= 11.0 since it covers most of the major features > > and should be pretty universally accessibly. > > > > 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. > > > > 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. > > > > 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. > > --- > > .clang-format | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 175 insertions(+) > > create mode 100644 .clang-format > > > > diff --git a/.clang-format b/.clang-format > > new file mode 100644 > > index 0000000000..fcb344ef70 > > --- /dev/null > > +++ b/.clang-format > > @@ -0,0 +1,175 @@ > > +# 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: > > +# > > +# Documentation/process/clang-format.rst > > That's not a valid path in the glibc source tree. > > > +# BasedOnStyle: GNU > > Why is this commented out? > > > +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 > > These regular expression do not seem to be relevant to glibc. Yeah. Will fix in v4. > > > +UseTab: Never > > I prefer not using tabs, but others disagree, and most of the source > files use tabs instead of 8 spaces (and not just for indentation). +1 but yeah the consistent style seems to be tabs so will update in v4. > > > +ForEachMacros: > > + - 'FOR_EACH_IMPL' > > + - 'list_for_each' > > + - 'list_for_each_prev' > > + - 'list_for_each_prev_safe' > > If this is a YAML file, do these duplicate FoEachMacros keys have any > effect? Only the second one is relevant to glibc. They all seem to have an effect. What do you mean only the second is relevant? I see all but `list_for_each_prev` used and `list_for_each_prev` appears to be defined so it could be used to make a loop.