|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: Clang-format configuration discussion - pt 2
On 24.11.2023 15:52, Luca Fancellu wrote: >> On 24 Nov 2023, at 12:47, Jan Beulich <jbeulich@xxxxxxxx> wrote: >> On 23.11.2023 15:47, Luca Fancellu wrote: >>> Let’s continue the discussion about clang-format configuration, this is >>> part 2, previous discussions are: >>> >>> - https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg00498.html >>> >>> You can find the serie introducing clang-format here: >>> https://patchwork.kernel.org/project/xen-devel/cover/20231031132304.2573924-1-luca.fancellu@xxxxxxx/ >>> and there is also a patch linked to my gitlab account where you can find >>> the output for the hypervisor code. >>> >>> For a full list of configurables and to find the possible values for them, >>> please refer to this page: >>> https://clang.llvm.org/docs/ClangFormatStyleOptions.html >>> >>> -------------------------------------------------------------------------------------------------------------------------------------------------- >>> >>> Our coding style doesn’t mention anything about alignment, shall we add a >>> new section? >>> I can send patches when we reach agreement on each of these rules. >>> >>> >>> QualifierAlignment: Custom >>> QualifierOrder: ['static', 'inline', 'const', 'volatile', 'type'] >>> >>> --- >>> For “QualifierAlignment” I chose Custom in order to apply in >>> “QualifierOrder” an order for the >>> qualifiers that match the current codebase, we could specify also “Leave” >>> in order to keep >>> them as they are. >> >> Where do attributes go in this sequence? > > I think function declaration/definition and variables. How does this relate to my question? I asked about the sequence of elements listed for QualifierOrder:, where attributes don't appear at all right now. >>> -------------------------------------------------------------------------------------------------------------------------------------------------- >>> >>> AlignAfterOpenBracket: Align >>> >>> --- >>> This one is to align function parameters that overflows the line length, I >>> chose to align them >>> to the open bracket to match the current codebase (hopefully) >>> >>> e.g.: >>> someLongFunction(argument1, >>> argument2); >> >> The above matches neither of the two generally permitted styles: >> >> someLongFunction(argument1, >> argument2); >> >> someLongFunction( >> argument1, >> argument2); >> >> Then again from its name I would infer this isn't just about function >> arguments? > > I think it applies to parameters and arguments of functions and macro, given > the description in the docs. > > I see your two snippets above but I’ve always found at least on arm a > predominance of > the style above for functions, so arguments aligned after the opening > bracket, for macros > there is a mix. The latter "above" refers to which form exactly? The one you originally spelled out, or the former of what my reply had? > I might be wrong though and so another opinion from another maintainer would > help. > > In any case we can choose among many value: > https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignafteropenbracket, > but when we do so, we need to stick to one permitted style only, the tool > don’t allow to specify more than one. On top of my earlier reply yet another reason perhaps that this tool then won't really fit our intended use. >>> -------------------------------------------------------------------------------------------------------------------------------------------------- >>> >>> AlignArrayOfStructures: Left >>> >>> --- >>> “When using initialization for an array of structs aligns the fields into >>> columns." >>> It’s important to say that even if we specify “None”, it is going to format >>> the data structure anyway, >>> I choose left, but clearly I’m open to suggestions. >> >> You don't say in which way it re-formats such constructs. > > Sure, taking as example an array of structure, xen/drivers/video/modelines.h, > > With AlignArrayOfStructures: None we have this below. > > diff --git a/xen/drivers/video/modelines.h b/xen/drivers/video/modelines.h > index 9cb7cdde055f..3ff23ef1f8a7 100644 > --- a/xen/drivers/video/modelines.h > +++ b/xen/drivers/video/modelines.h > @@ -42,36 +42,36 @@ struct modeline { > }; > > struct modeline __initdata videomodes[] = { > - { "640x480@60", 25175, 640, 16, 96, 48, 480, 11, 2, 31 }, > - { "640x480@72", 31500, 640, 24, 40, 128, 480, 9, 3, 28 }, > - { "640x480@75", 31500, 640, 16, 96, 48, 480, 11, 2, 32 }, > - { "640x480@85", 36000, 640, 32, 48, 112, 480, 1, 3, 25 }, > - { "800x600@56", 38100, 800, 32, 128, 128, 600, 1, 4, 14 }, > - { "800x600@60", 40000, 800, 40, 128, 88 , 600, 1, 4, 23 }, > - { "800x600@72", 50000, 800, 56, 120, 64 , 600, 37, 6, 23 }, > - { "800x600@75", 49500, 800, 16, 80, 160, 600, 1, 2, 21 }, > - { "800x600@85", 56250, 800, 32, 64, 152, 600, 1, 3, 27 }, > - { "1024x768@60", 65000, 1024, 24, 136, 160, 768, 3, 6, 29 }, > - { "1024x768@70", 75000, 1024, 24, 136, 144, 768, 3, 6, 29 }, > - { "1024x768@75", 78750, 1024, 16, 96, 176, 768, 1, 3, 28 }, > - { "1024x768@85", 94500, 1024, 48, 96, 208, 768, 1, 3, 36 }, > - { "1280x1024@60", 108000, 1280, 48, 112, 248, 1024, 1, 3, 38 }, > - { "1280x1024@75", 135000, 1280, 16, 144, 248, 1024, 1, 3, 38 }, > - { "1280x1024@85", 157500, 1280, 64, 160, 224, 1024, 1, 3, 44 }, > - { "1400x1050@60", 122610, 1400, 88, 152, 240, 1050, 1, 3, 33 }, > - { "1400x1050@75", 155850, 1400, 96, 152, 248, 1050, 1, 3, 42 }, > - { "1600x1200@60", 162000, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > - { "1600x1200@65", 175500, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > - { "1600x1200@70", 189000, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > - { "1600x1200@75", 202500, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > - { "1600x1200@85", 229500, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > - { "1792x1344@60", 204800, 1792, 128, 200, 328, 1344, 1, 3, 46 }, > - { "1792x1344@75", 261000, 1792, 96, 216, 352, 1344, 1, 3, 69 }, > - { "1856x1392@60", 218300, 1856, 96, 224, 352, 1392, 1, 3, 43 }, > - { "1856x1392@75", 288000, 1856, 128, 224, 352, 1392, 1, 3, 104 > }, > - { "1920x1200@75", 193160, 1920, 128, 208, 336, 1200, 1, 3, 38 }, > - { "1920x1440@60", 234000, 1920, 128, 208, 344, 1440, 1, 3, 56 }, > - { "1920x1440@75", 297000, 1920, 144, 224, 352, 1440, 1, 3, 56 }, > + { "640x480@60", 25175, 640, 16, 96, 48, 480, 11, 2, 31 }, > + { "640x480@72", 31500, 640, 24, 40, 128, 480, 9, 3, 28 }, > + { "640x480@75", 31500, 640, 16, 96, 48, 480, 11, 2, 32 }, > + { "640x480@85", 36000, 640, 32, 48, 112, 480, 1, 3, 25 }, > + { "800x600@56", 38100, 800, 32, 128, 128, 600, 1, 4, 14 }, > + { "800x600@60", 40000, 800, 40, 128, 88, 600, 1, 4, 23 }, > + { "800x600@72", 50000, 800, 56, 120, 64, 600, 37, 6, 23 }, > + { "800x600@75", 49500, 800, 16, 80, 160, 600, 1, 2, 21 }, > + { "800x600@85", 56250, 800, 32, 64, 152, 600, 1, 3, 27 }, > + { "1024x768@60", 65000, 1024, 24, 136, 160, 768, 3, 6, 29 }, > + { "1024x768@70", 75000, 1024, 24, 136, 144, 768, 3, 6, 29 }, > + { "1024x768@75", 78750, 1024, 16, 96, 176, 768, 1, 3, 28 }, > + { "1024x768@85", 94500, 1024, 48, 96, 208, 768, 1, 3, 36 }, > + { "1280x1024@60", 108000, 1280, 48, 112, 248, 1024, 1, 3, 38 }, > + { "1280x1024@75", 135000, 1280, 16, 144, 248, 1024, 1, 3, 38 }, > + { "1280x1024@85", 157500, 1280, 64, 160, 224, 1024, 1, 3, 44 }, > + { "1400x1050@60", 122610, 1400, 88, 152, 240, 1050, 1, 3, 33 }, > + { "1400x1050@75", 155850, 1400, 96, 152, 248, 1050, 1, 3, 42 }, > + { "1600x1200@60", 162000, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > + { "1600x1200@65", 175500, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > + { "1600x1200@70", 189000, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > + { "1600x1200@75", 202500, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > + { "1600x1200@85", 229500, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > + { "1792x1344@60", 204800, 1792, 128, 200, 328, 1344, 1, 3, 46 }, > + { "1792x1344@75", 261000, 1792, 96, 216, 352, 1344, 1, 3, 69 }, > + { "1856x1392@60", 218300, 1856, 96, 224, 352, 1392, 1, 3, 43 }, > + { "1856x1392@75", 288000, 1856, 128, 224, 352, 1392, 1, 3, 104 }, > + { "1920x1200@75", 193160, 1920, 128, 208, 336, 1200, 1, 3, 38 }, > + { "1920x1440@60", 234000, 1920, 128, 208, 344, 1440, 1, 3, 56 }, > + { "1920x1440@75", 297000, 1920, 144, 224, 352, 1440, 1, 3, 56 }, > }; > > #endif > > With AlignArrayOfStructures: Left we have (I noticed there might be a small > bug in clang-format since the first entry has no space after the opening > curly bracket): > > diff --git a/xen/drivers/video/modelines.h b/xen/drivers/video/modelines.h > index 9cb7cdde055f..1afe725dcb4c 100644 > --- a/xen/drivers/video/modelines.h > +++ b/xen/drivers/video/modelines.h > @@ -42,36 +42,36 @@ struct modeline { > }; > > struct modeline __initdata videomodes[] = { > - { "640x480@60", 25175, 640, 16, 96, 48, 480, 11, 2, 31 }, > - { "640x480@72", 31500, 640, 24, 40, 128, 480, 9, 3, 28 }, > - { "640x480@75", 31500, 640, 16, 96, 48, 480, 11, 2, 32 }, > - { "640x480@85", 36000, 640, 32, 48, 112, 480, 1, 3, 25 }, > - { "800x600@56", 38100, 800, 32, 128, 128, 600, 1, 4, 14 }, > - { "800x600@60", 40000, 800, 40, 128, 88 , 600, 1, 4, 23 }, > - { "800x600@72", 50000, 800, 56, 120, 64 , 600, 37, 6, 23 }, > - { "800x600@75", 49500, 800, 16, 80, 160, 600, 1, 2, 21 }, > - { "800x600@85", 56250, 800, 32, 64, 152, 600, 1, 3, 27 }, > - { "1024x768@60", 65000, 1024, 24, 136, 160, 768, 3, 6, 29 }, > - { "1024x768@70", 75000, 1024, 24, 136, 144, 768, 3, 6, 29 }, > - { "1024x768@75", 78750, 1024, 16, 96, 176, 768, 1, 3, 28 }, > - { "1024x768@85", 94500, 1024, 48, 96, 208, 768, 1, 3, 36 }, > - { "1280x1024@60", 108000, 1280, 48, 112, 248, 1024, 1, 3, 38 }, > - { "1280x1024@75", 135000, 1280, 16, 144, 248, 1024, 1, 3, 38 }, > - { "1280x1024@85", 157500, 1280, 64, 160, 224, 1024, 1, 3, 44 }, > - { "1400x1050@60", 122610, 1400, 88, 152, 240, 1050, 1, 3, 33 }, > - { "1400x1050@75", 155850, 1400, 96, 152, 248, 1050, 1, 3, 42 }, > - { "1600x1200@60", 162000, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > - { "1600x1200@65", 175500, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > - { "1600x1200@70", 189000, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > - { "1600x1200@75", 202500, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > - { "1600x1200@85", 229500, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > - { "1792x1344@60", 204800, 1792, 128, 200, 328, 1344, 1, 3, 46 }, > - { "1792x1344@75", 261000, 1792, 96, 216, 352, 1344, 1, 3, 69 }, > - { "1856x1392@60", 218300, 1856, 96, 224, 352, 1392, 1, 3, 43 }, > - { "1856x1392@75", 288000, 1856, 128, 224, 352, 1392, 1, 3, 104 > }, > - { "1920x1200@75", 193160, 1920, 128, 208, 336, 1200, 1, 3, 38 }, > - { "1920x1440@60", 234000, 1920, 128, 208, 344, 1440, 1, 3, 56 }, > - { "1920x1440@75", 297000, 1920, 144, 224, 352, 1440, 1, 3, 56 }, > + {"640x480@60", 25175, 640, 16, 96, 48, 480, 11, 2, 31 }, > + { "640x480@72", 31500, 640, 24, 40, 128, 480, 9, 3, 28 }, > + { "640x480@75", 31500, 640, 16, 96, 48, 480, 11, 2, 32 }, > + { "640x480@85", 36000, 640, 32, 48, 112, 480, 1, 3, 25 }, > + { "800x600@56", 38100, 800, 32, 128, 128, 600, 1, 4, 14 }, > + { "800x600@60", 40000, 800, 40, 128, 88, 600, 1, 4, 23 }, > + { "800x600@72", 50000, 800, 56, 120, 64, 600, 37, 6, 23 }, > + { "800x600@75", 49500, 800, 16, 80, 160, 600, 1, 2, 21 }, > + { "800x600@85", 56250, 800, 32, 64, 152, 600, 1, 3, 27 }, > + { "1024x768@60", 65000, 1024, 24, 136, 160, 768, 3, 6, 29 }, > + { "1024x768@70", 75000, 1024, 24, 136, 144, 768, 3, 6, 29 }, > + { "1024x768@75", 78750, 1024, 16, 96, 176, 768, 1, 3, 28 }, > + { "1024x768@85", 94500, 1024, 48, 96, 208, 768, 1, 3, 36 }, > + { "1280x1024@60", 108000, 1280, 48, 112, 248, 1024, 1, 3, 38 }, > + { "1280x1024@75", 135000, 1280, 16, 144, 248, 1024, 1, 3, 38 }, > + { "1280x1024@85", 157500, 1280, 64, 160, 224, 1024, 1, 3, 44 }, > + { "1400x1050@60", 122610, 1400, 88, 152, 240, 1050, 1, 3, 33 }, > + { "1400x1050@75", 155850, 1400, 96, 152, 248, 1050, 1, 3, 42 }, > + { "1600x1200@60", 162000, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > + { "1600x1200@65", 175500, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > + { "1600x1200@70", 189000, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > + { "1600x1200@75", 202500, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > + { "1600x1200@85", 229500, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > + { "1792x1344@60", 204800, 1792, 128, 200, 328, 1344, 1, 3, 46 }, > + { "1792x1344@75", 261000, 1792, 96, 216, 352, 1344, 1, 3, 69 }, > + { "1856x1392@60", 218300, 1856, 96, 224, 352, 1392, 1, 3, 43 }, > + { "1856x1392@75", 288000, 1856, 128, 224, 352, 1392, 1, 3, 104}, > + { "1920x1200@75", 193160, 1920, 128, 208, 336, 1200, 1, 3, 38 }, > + { "1920x1440@60", 234000, 1920, 128, 208, 344, 1440, 1, 3, 56 }, > + { "1920x1440@75", 297000, 1920, 144, 224, 352, 1440, 1, 3, 56 }, > }; > > #endif > > With AlignArrayOfStructures: Right we have: > > diff --git a/xen/drivers/video/modelines.h b/xen/drivers/video/modelines.h > index 9cb7cdde055f..539ab7c12d00 100644 > --- a/xen/drivers/video/modelines.h > +++ b/xen/drivers/video/modelines.h > @@ -42,36 +42,36 @@ struct modeline { > }; > > struct modeline __initdata videomodes[] = { > - { "640x480@60", 25175, 640, 16, 96, 48, 480, 11, 2, 31 }, > - { "640x480@72", 31500, 640, 24, 40, 128, 480, 9, 3, 28 }, > - { "640x480@75", 31500, 640, 16, 96, 48, 480, 11, 2, 32 }, > - { "640x480@85", 36000, 640, 32, 48, 112, 480, 1, 3, 25 }, > - { "800x600@56", 38100, 800, 32, 128, 128, 600, 1, 4, 14 }, > - { "800x600@60", 40000, 800, 40, 128, 88 , 600, 1, 4, 23 }, > - { "800x600@72", 50000, 800, 56, 120, 64 , 600, 37, 6, 23 }, > - { "800x600@75", 49500, 800, 16, 80, 160, 600, 1, 2, 21 }, > - { "800x600@85", 56250, 800, 32, 64, 152, 600, 1, 3, 27 }, > - { "1024x768@60", 65000, 1024, 24, 136, 160, 768, 3, 6, 29 }, > - { "1024x768@70", 75000, 1024, 24, 136, 144, 768, 3, 6, 29 }, > - { "1024x768@75", 78750, 1024, 16, 96, 176, 768, 1, 3, 28 }, > - { "1024x768@85", 94500, 1024, 48, 96, 208, 768, 1, 3, 36 }, > - { "1280x1024@60", 108000, 1280, 48, 112, 248, 1024, 1, 3, 38 }, > - { "1280x1024@75", 135000, 1280, 16, 144, 248, 1024, 1, 3, 38 }, > - { "1280x1024@85", 157500, 1280, 64, 160, 224, 1024, 1, 3, 44 }, > - { "1400x1050@60", 122610, 1400, 88, 152, 240, 1050, 1, 3, 33 }, > - { "1400x1050@75", 155850, 1400, 96, 152, 248, 1050, 1, 3, 42 }, > - { "1600x1200@60", 162000, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > - { "1600x1200@65", 175500, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > - { "1600x1200@70", 189000, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > - { "1600x1200@75", 202500, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > - { "1600x1200@85", 229500, 1600, 64, 192, 304, 1200, 1, 3, 46 }, > - { "1792x1344@60", 204800, 1792, 128, 200, 328, 1344, 1, 3, 46 }, > - { "1792x1344@75", 261000, 1792, 96, 216, 352, 1344, 1, 3, 69 }, > - { "1856x1392@60", 218300, 1856, 96, 224, 352, 1392, 1, 3, 43 }, > - { "1856x1392@75", 288000, 1856, 128, 224, 352, 1392, 1, 3, 104 > }, > - { "1920x1200@75", 193160, 1920, 128, 208, 336, 1200, 1, 3, 38 }, > - { "1920x1440@60", 234000, 1920, 128, 208, 344, 1440, 1, 3, 56 }, > - { "1920x1440@75", 297000, 1920, 144, 224, 352, 1440, 1, 3, 56 }, > + { "640x480@60", 25175, 640, 16, 96, 48, 480, 11, 2, 31}, > + { "640x480@72", 31500, 640, 24, 40, 128, 480, 9, 3, 28}, > + { "640x480@75", 31500, 640, 16, 96, 48, 480, 11, 2, 32}, > + { "640x480@85", 36000, 640, 32, 48, 112, 480, 1, 3, 25}, > + { "800x600@56", 38100, 800, 32, 128, 128, 600, 1, 4, 14}, > + { "800x600@60", 40000, 800, 40, 128, 88, 600, 1, 4, 23}, > + { "800x600@72", 50000, 800, 56, 120, 64, 600, 37, 6, 23}, > + { "800x600@75", 49500, 800, 16, 80, 160, 600, 1, 2, 21}, > + { "800x600@85", 56250, 800, 32, 64, 152, 600, 1, 3, 27}, > + { "1024x768@60", 65000, 1024, 24, 136, 160, 768, 3, 6, 29}, > + { "1024x768@70", 75000, 1024, 24, 136, 144, 768, 3, 6, 29}, > + { "1024x768@75", 78750, 1024, 16, 96, 176, 768, 1, 3, 28}, > + { "1024x768@85", 94500, 1024, 48, 96, 208, 768, 1, 3, 36}, > + {"1280x1024@60", 108000, 1280, 48, 112, 248, 1024, 1, 3, 38}, > + {"1280x1024@75", 135000, 1280, 16, 144, 248, 1024, 1, 3, 38}, > + {"1280x1024@85", 157500, 1280, 64, 160, 224, 1024, 1, 3, 44}, > + {"1400x1050@60", 122610, 1400, 88, 152, 240, 1050, 1, 3, 33}, > + {"1400x1050@75", 155850, 1400, 96, 152, 248, 1050, 1, 3, 42}, > + {"1600x1200@60", 162000, 1600, 64, 192, 304, 1200, 1, 3, 46}, > + {"1600x1200@65", 175500, 1600, 64, 192, 304, 1200, 1, 3, 46}, > + {"1600x1200@70", 189000, 1600, 64, 192, 304, 1200, 1, 3, 46}, > + {"1600x1200@75", 202500, 1600, 64, 192, 304, 1200, 1, 3, 46}, > + {"1600x1200@85", 229500, 1600, 64, 192, 304, 1200, 1, 3, 46}, > + {"1792x1344@60", 204800, 1792, 128, 200, 328, 1344, 1, 3, 46}, > + {"1792x1344@75", 261000, 1792, 96, 216, 352, 1344, 1, 3, 69}, > + {"1856x1392@60", 218300, 1856, 96, 224, 352, 1392, 1, 3, 43}, > + {"1856x1392@75", 288000, 1856, 128, 224, 352, 1392, 1, 3, 104}, > + {"1920x1200@75", 193160, 1920, 128, 208, 336, 1200, 1, 3, 38}, > + {"1920x1440@60", 234000, 1920, 128, 208, 344, 1440, 1, 3, 56}, > + {"1920x1440@75", 297000, 1920, 144, 224, 352, 1440, 1, 3, 56}, > }; > > #endif None of which improve what we have at present, imo. Yet another reason not to use this tool then, I would say. Jan
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |