From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id qxiQHU6HmmBVPgAAWB0awg (envelope-from ) for ; Tue, 11 May 2021 09:31:58 -0400 Received: by simark.ca (Postfix, from userid 112) id 6A97D1F11C; Tue, 11 May 2021 09:31:58 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 330131E01F for ; Tue, 11 May 2021 09:31:57 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E131C388F03D; Tue, 11 May 2021 13:31:56 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E131C388F03D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1620739916; bh=YrLaLtef9icZiKQde72B3jAqrJs6uaaPtSggYtHt1mc=; h=Subject:In-Reply-To:Date:References:To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=Rr2KVoH1aD48i+xIzpQ1zTY4TvW4DFJoOuRS5FSa7Z0afolBBVUDihdtEn3D14ScL ZISR1ZMid0a1nTkr9D1ak4s+ZhaAPjPusCNRznbX1MBst5owpc8NbPGvGPDNFMahVu Vro47i3NLkEvtzCaKnaOBhL+o6Mmnn6brqeWAYtM= Received: from mail-wm1-x32c.google.com (mail-wm1-x32c.google.com [IPv6:2a00:1450:4864:20::32c]) by sourceware.org (Postfix) with ESMTPS id DF7A6384B110 for ; Tue, 11 May 2021 13:31:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org DF7A6384B110 Received: by mail-wm1-x32c.google.com with SMTP id g65so11140139wmg.2 for ; Tue, 11 May 2021 06:31:52 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=YrLaLtef9icZiKQde72B3jAqrJs6uaaPtSggYtHt1mc=; b=lBi4CVgLfs3+i0Pzsq87ExJGK/bDf41DDt5lgrgm4FY8EYvNYL6A35HfbcY2yw1BKX ehAkbrtFN903qtmg9D5XzApvwHiQ72659CFoQnptKwZgxHzNwNLL+8bh5EWOiTebEgZ7 y/bAY6HvpsoDGFkBdAzfJoAyDNGdCcyHYObTM1QUaAtVvb2aTJabjUDL3lQl8xWvXbNi HKfYLv4b56lxGvrXlv9gAFZIQRu/uLNB3vfCxsdwNVqn1SLgXDwgC9GJXs6MW4lPL78e +ue+6i7Waj295Wdx9MNT/0ACFGdHH8mrGNHLZot1I0X33AuSRA390/VVIJWgCXaF5sBu Jn5g== X-Gm-Message-State: AOAM532iOHQdRCxA3rVahe3SQeP8e6ApTjYqtQEtTOWl3AIFBWwPEijb YUu81JTuUXJqxBoDlyCUr2AWS06g38qWlZmr X-Google-Smtp-Source: ABdhPJyLUhIFV1DVzGs/hMtVkN0lhHI/71c3vMmj9dK9KNf6hAGlg49bTBvJ3z/DZzqoYemtDH374g== X-Received: by 2002:a05:600c:da:: with SMTP id u26mr5501999wmm.8.1620739911803; Tue, 11 May 2021 06:31:51 -0700 (PDT) Received: from smtpclient.apple (cpc159317-cmbg20-2-0-cust151.5-4.cable.virginm.net. [81.111.29.152]) by smtp.gmail.com with ESMTPSA id v13sm28371197wrt.65.2021.05.11.06.31.51 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 11 May 2021 06:31:51 -0700 (PDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.80.0.2.43\)) Subject: Re: Using clang-format In-Reply-To: <9e485731-875c-98e2-1894-6b1a45efa5c5@polymtl.ca> Date: Tue, 11 May 2021 14:31:47 +0100 Content-Transfer-Encoding: quoted-printable Message-Id: References: <87lf8p9pwg.fsf@tromey.com> <9e485731-875c-98e2-1894-6b1a45efa5c5@polymtl.ca> To: Simon Marchi X-Mailer: Apple Mail (2.3654.80.0.2.43) X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Marco Barisione via Gdb-patches Reply-To: Marco Barisione Cc: Tom Tromey , GDB patches mailing list Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" On 11 May 2021, at 03:57, Simon Marchi via Gdb-patches = wrote: >=20 > On 2021-05-10 10:55 p.m., Simon Marchi via Gdb-patches wrote: >> On 2021-05-08 12:00 p.m., Tom Tromey wrote: >>>>>>>> "Simon" =3D=3D Simon Marchi via Gdb-patches = writes: >>>=20 >>> Simon> A few people I talked to about this and I have good = experience >>> Simon> with the tool black to auto-format Python code. >>>=20 >>> Inspired by this, I took another look at clang-format. If we could = use >>> this, I think it would be a decent productivity improvement, because = it >>> would mean contributors would no longer need to worry about the = minutiae >>> of the GNU style. We could also stop reviewing this. >>=20 >> I would love this. As an occasional contributor I=E2=80=99d love this. Getting the correct style right is complicated if you spend most of your time developing other projects with other coding styles. It=E2=80=99s even worse for cases that are not trivial as GDB=E2=80=99s = codebase is occasionally inconsistent so there=E2=80=99s nowhere where to take = inspiration from. >> I noticed this: >>=20 >> btrace_block (CORE_ADDR begin, CORE_ADDR end) : begin (begin), end = (end) >> { >> /* Nothing. */ >> } >>=20 >> The initializer list is put on the same line as the constructor's >> prototype. If there's a way to tell it that we don't want that, it >> would be closer to our style. You can use BreakConstructorInitializers. >>> Another possible problem is that, unlike with 'black', ensuring that >>> everyone has the same version is a pain. >>=20 >> Indeed. It's perhaps easy for me to say, because I get to choose = what >> Linux distro and version I work on (so I opt for something recent), = but >> I would still lean towards just following whatever the current latest >> stable version is. There might be new options in the latest stable >> version we want to use, it would be nice not to have to wait years >> before we can use them. And I suppose there's a not too painful way = to >> get it for all the major distros out there (and for Windows and = macOS, >> there are binary releases). And you can always run it in Docker or >> something. >>=20 >> But we would probably encourage people to use git-clang-format, which >> only reformats the lines you touched. So, it wouldn't be a big = problem >> for people to use different versions, as it wouldn't create spurious >> changes elsewhere in the file. The differences in formatting would = be >> in lines you touch anyway. >>=20 >> Speaking of git-clang-format: from my experience, it is not super = cool >> when the code base isn't already formatted with the target formatting >> style. It causes discrepancies in style, because you have a mix of = new >> style (whatever lines were touched) and old style (whatever lines = were >> not touched) which can be annoying and distracting. Or, it causes >> slightly bigger diffs than you would get otherwise. For example, I >> ran git-clang-format on this commit here: >>=20 >> = https://sourceware.org/git/?p=3Dbinutils-gdb.git;a=3Dcommit;h=3D10e578d7e0= 0d >>=20 >> ... to see what would have been the resulting patch had the author = used >> git-clang-format. This would have been the patch: >>=20 >> https://pastebin.com/pxrZtnDU >>=20 >> If you check the changes in mi/mi-cmd-break.c, you can see that >> git-clang-format re-formatted the whole enum and array initializer. >> Which is nice in itself, the resulting formatting is nice. But, the >> patch would be a bit harder to read. >>=20 >> The other option is to start by doing a big re-format and then tell >> people to use git-clang-format. This way, there might be some small >> incoherences here and there due to different versions, but I think >> they'll be insignificant. >>=20 >> With the Python code, we did a massive re-format. I don't think it = did >> / will cause a big inconvenience, because there aren't many patches = to >> the Python code. But for the C++ code, it might be different, as = there >> are more patches in everyone's pipeline that touch that code. Personally, I=E2=80=99d be a bit worried about dealing with difficult = conflicts manually, but there are solutions for that: 1. Rebase your branch to the point just before reformatting: $ git rebase 2. Rebase on top of the format changes but preferring your local changes: $ git rebase --strategy=3Drecursive \ --strategy-option=3Dtheirs 3. Finally reformat all of your code you just committed and amend your previous commits. At work we decided not to reformat the whole C/C++ code base (while we did for Python). Instead, we use some git hooks I wrote which reformat only what you are committing. See https://github.com/barisione/clang-format-hooks. While I wrote these scripts and would be happy for others to use them, I think that (in GDB=E2=80=99s case) it would be easier to just reformat everything and deal with the short-term pain.=20 --=20 Marco Barisione