Bug 468401 - [PATCH] Add a style file for clang-format
Summary: [PATCH] Add a style file for clang-format
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (other bugs)
Version First Reported In: unspecified
Platform: Other Linux
: NOR minor
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-04-11 19:27 UTC by Petr Pavlu
Modified: 2023-04-17 20:09 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments
0001-Add-a-style-file-for-clang-format.patch (1.02 KB, patch)
2023-04-11 19:27 UTC, Petr Pavlu
Details
v2-0001-Add-a-style-file-for-clang-format.patch (2.39 KB, patch)
2023-04-16 14:04 UTC, Petr Pavlu
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Petr Pavlu 2023-04-11 19:27:23 UTC
Created attachment 158020 [details]
0001-Add-a-style-file-for-clang-format.patch

clang-format [1] is a tool for formatting C/C++ code. It is in use by a number
of projects to help to keep their code style consistent.

I've been using this tool with a customized style to format code of the
Valgrind port for RISC-V [2]. My experience is that it generally produces good
results and keeps the code nicely formatted, certainly better than I would be
able to do manually myself.

The customized configuration tries to match what appears to be a dominant
style used by the Valgrind codebase.

Examples:
* coregrind/m_sigframe/sigframe-riscv64-linux.c [3]: Coregrind code to show
  that the VG_() and ML_() specific macros used by Valgrind get sensibly
  handled.
* VEX/priv/guest_riscv64_toIR.c [4]: Lengthier VEX code.

The proposed patch adds this configuration file as I think having an option to
use clang-format to format Valgrind code could be useful for others too. For
instance, using the tool can make one-off contributions easier and could save
some discussions about code style during code reviews.

[1] https://clang.llvm.org/docs/ClangFormat.html
[2] https://github.com/petrpavlu/valgrind-riscv64
[3] https://github.com/petrpavlu/valgrind-riscv64/blob/71272b252977fe52f03ea4fa8306b457b098cca5/coregrind/m_sigframe/sigframe-riscv64-linux.c
[4] https://github.com/petrpavlu/valgrind-riscv64/blob/71272b252977fe52f03ea4fa8306b457b098cca5/VEX/priv/guest_riscv64_toIR.c
Comment 1 Paul Floyd 2023-04-12 08:12:48 UTC
Thanks Petr this looks like a good start. Do you think that this will also need TypenameMacros for the tool 'namespace' macros (MC_ etc.)?

Any thoughts on adding this to git hooks?

I think it would also be a good idea to add a section to README_DEVELOPERS (difficult to be totally comprehensive, but things like which package provides clang-format, how to use it, IDE integration ...)
Comment 2 Petr Pavlu 2023-04-16 14:04:39 UTC
Created attachment 158146 [details]
v2-0001-Add-a-style-file-for-clang-format.patch
Comment 3 Petr Pavlu 2023-04-16 14:07:48 UTC
(In reply to Paul Floyd from comment #1)
> Thanks Petr this looks like a good start. Do you think that this will also
> need TypenameMacros for the tool 'namespace' macros (MC_ etc.)?

Missed these, added in v2 of the patch.

> Any thoughts on adding this to git hooks?

Not sure, it might be hard to tell clang-format what it should exactly do from a Git hook and it has some potential to annoy people that don't want to use the tool.

I would start simply and only add a configuration file in the root directory. This gives folks that want to use this tool an easy way to do so.

> I think it would also be a good idea to add a section to README_DEVELOPERS
> (difficult to be totally comprehensive, but things like which package
> provides clang-format, how to use it, IDE integration ...)

Makes sense, v2 adds some basic information in README_DEVELOPERS and a reference to the upstream documentation for integration with text editors and IDEs.
Comment 4 Paul Floyd 2023-04-17 20:09:53 UTC
commit 1b3430761f6bda43b8187dbd342b34cb5c99df3f (HEAD -> master, origin/master, origin/HEAD)
Author: Paul Floyd <pjfloyd@wanadoo.fr>
Date:   Mon Apr 17 22:05:30 2023 +0200

    Bug 468401 - [PATCH] Add a style file for clang-format
    
    Patch submitted by:
            Petr Pavlu <petr.pavlu@dagobah.cz>