Bug 468401

Summary:	[PATCH] Add a style file for clang-format
Product:	[Developer tools] valgrind	Reporter:	Petr Pavlu <petr.pavlu>
Component:	general	Assignee:	Julian Seward <jseward>
Status:	RESOLVED FIXED
Severity:	minor	CC:	pjfloyd
Priority:	NOR
Version First Reported In:	unspecified
Target Milestone:	---
Platform:	Other
OS:	Linux
Latest Commit:		Version Fixed/Implemented In:
Sentry Crash Report:
Attachments:	0001-Add-a-style-file-for-clang-format.patch v2-0001-Add-a-style-file-for-clang-format.patch

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>