GSoC - Code Formatting and Style Checking for RTEMS score

Fri Jun 4 05:24:04 UTC 2021

Hello everyone,

I applied the configuration Sebastian used and ran clang-format on
cpukit/score/src/threadqenque.c and so far these are the differences I
could notice...
Below are some example areas in the code you can spot the differences:

   - In line 68, the ")" at the end of the parameter list needs to be in a
   new row, but this doesn't seem to be supported in clang-format.
   - In line 142, if the function call is split into multiple rows, the
   ");" should always be in a new row.
   - In line 201-202, we can see that the "*" of the pointers are not
   aligned to the right.

You can check out the formatted file here - https://pastebin.com/nDBrSSCP

On Tue, Jun 1, 2021 at 5:36 PM Gedare Bloom <gedare at rtems.org> wrote:

> On Mon, May 31, 2021 at 2:59 PM Ida Delphine <idadelm at gmail.com> wrote:
> >
> > Hi Gedare,
> >
> > With regards to your comment on discord on me looking for a tool that
> works on both patches and source files, it turns out clang-format has that
> functionality already. Here's what I found -
> https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting
> >
> > Does it match what you have in mind?
> >
> Yes. I think we would want to not use the `-i` option but instead pass
> through and check the changes. I don't think we should rewrite the
> patches themselves, but instead we want to use a tool that can be used
> to check and approve the style of submitted patches. You might need to
> write a modified version of the clang-format-diff.py to use as a
> "checker" with ability to provide exceptions to the rules.
>
> Gedare
>
> > On Thu, May 13, 2021 at 3:49 PM Gedare Bloom <gedare at rtems.org> wrote:
> >>
> >> On Wed, May 12, 2021 at 2:18 PM Ida Delphine <idadelm at gmail.com> wrote:
> >> >
> >> > Hello everyone,
> >> > Still waiting for some feedback :)
> >> >
> >> > Cheers,
> >> > Ida.
> >> >
> >> > On Mon, 10 May 2021, 5:59 am Ida Delphine, <idadelm at gmail.com> wrote:
> >> >>
> >> >> Hello everyone,
> >> >> Went through some previous emails and it turns out Sebastian already
> came up with a configuration for clang format which works well for RTEMS
> except for the fact that some configurations haven't been implemented into
> clang-format yet. Using
> >> >>
> >> >> AlignConsecutiveDeclarations: false
> >> >> PointerAlignment: Right
> >> >>
> >> >> Doesn't seem to work.
> >> >> For example in the cpukit/score/src/threadq.c file, something like
> >> >>
> >> >> RTEMS_STATIC_ASSERT(
> >> >> offsetof( Thread_queue_Syslock_queue, Queue.name )
> >> >> == offsetof( struct _Thread_queue_Queue, _name ),
> >> >> THREAD_QUEUE_SYSLOCK_QUEUE_NAME
> >> >> );
> >> >>
> >> >> RTEMS_STATIC_ASSERT(
> >> >> sizeof( Thread_queue_Syslock_queue )
> >> >> == sizeof( struct _Thread_queue_Queue ),
> >> >> THREAD_QUEUE_SYSLOCK_QUEUE_SIZE
> >> >> );
> >> >>
> >> >> #if defined(RTEMS_SMP)
> >> >> void _Thread_queue_Do_acquire_critical(
> >> >> Thread_queue_Control *the_thread_queue,
> >> >> ISR_lock_Context *lock_context
> >> >> )
> >> >> {
> >> >> _Thread_queue_Queue_acquire_critical(
> >> >> &the_thread_queue->Queue,
> >> >> &the_thread_queue->Lock_stats,
> >> >> lock_context
> >> >> );
> >> >>
> >> >> becomes this after using the given configuration
> >> >>
> >> >> RTEMS_STATIC_ASSERT(sizeof(Thread_queue_Syslock_queue) ==
> >> >> sizeof(struct _Thread_queue_Queue),
> >> >> THREAD_QUEUE_SYSLOCK_QUEUE_SIZE);
> >> >>
> >> >> #if defined(RTEMS_SMP)
> >> >> void _Thread_queue_Do_acquire_critical(Thread_queue_Control
> *the_thread_queue,
> >> >> ISR_lock_Context *lock_context) {
> >> >> _Thread_queue_Queue_acquire_critical(
> >> >> &the_thread_queue->Queue, &the_thread_queue->Lock_stats,
> lock_context);
> >> >>
> >> >> Everything seems manageable except for this alignment issue...
> >> >> This also throws more light on the changes using clang-format (
> https://lists.rtems.org/pipermail/devel/2018-December/024145.html)
> >> >>
> >> I think we're willing to concede the pointer alignment. However, it
> >> would be worth spending some time to see if
> >> https://reviews.llvm.org/D27651 can be made to work. The current state
> >> of the code would need to be compared to the patch on that review
> >> board.
> >>
> >> Beyond that, documenting the clang-format options to use is next, and
> >> then identifying a plan how to invoke clang-format during a git
> >> workflow is needed.
> >>
> >> >> On Thu, May 6, 2021 at 8:05 PM Joel Sherrill <joel at rtems.org> wrote:
> >> >>>
> >> >>>
> >> >>>
> >> >>> On Thu, May 6, 2021 at 12:47 PM Christian Mauderer <
> oss at c-mauderer.de> wrote:
> >> >>>>
> >> >>>> Hello Ida and Gedare,
> >> >>>>
> >> >>>> On 06/05/2021 06:26, Gedare Bloom wrote:
> >> >>>> > hi Ida,
> >> >>>> >
> >> >>>> > On Wed, May 5, 2021 at 3:21 PM Ida Delphine <idadelm at gmail.com>
> wrote:
> >> >>>> >>
> >> >>>> >> Hello everyone,
> >> >>>> >>
> >> >>>> >> Regarding this project (https://devel.rtems.org/ticket/3860) I
> went with clang-format as we all agreed. I have tested it on some "score"
> files and it made some changes which I don't think are very much in line
> with the RTEMS coding style. However, it wasn't really clear if we will
> chage the RTEMS coding style or try to make changes to clang-format to fit
> the style.
> >> >>>> >> Please will love to know the best option.
> >> >>>> >>
> >> >>>> > We will likely need to consider our choices carefully. If we can
> find
> >> >>>> > a suitably close style that is already well-supported by clang,
> and
> >> >>>> > get consensus from the maintainers on a change, then that might
> be the
> >> >>>> > best route forward.
> >> >>>>
> >> >>>> +1
> >> >>>>
> >> >>>> > I think the first thing to do is take the examples
> >> >>>> > that have been shown by Sebastian that are "close" but not quite
> >> >>>> > perfect, and identify the cases where they differ with RTEMS
> style in
> >> >>>> > order to present for discussion here. If consensus can't be
> reached to
> >> >>>> > change the style, then we would need to have a plan for how to
> improve
> >> >>>> > the existing tools for what we have.
> >> >>>>
> >> >>>> I also found the following tool quite useful to play with the clang
> >> >>>> style config:
> >> >>>>
> >> >>>> https://zed0.co.uk/clang-format-configurator/
> >> >>>>
> >> >>>> Maybe it can help a bit to find out what certain options mean.
> >> >>>>
> >> >>>> >
> >> >>>> > However, I think there is interest in doing less work on the tool
> >> >>>> > side, and more work on how to integrate it into our workflows
> better.
> >> >>>>
> >> >>>> +1
> >> >>>
> >> >>>
> >> >>> I agree with all of this from the student perspective. But we will
> have
> >> >>> to come to some agreement on a machine producible format to
> >> >>> be able to use the integration. A report on what doesn't match would
> >> >>> give us something to chew on while Ida works the integration.
> >> >>>
> >> >>> --joel
> >> >>>>
> >> >>>>
> >> >>>> >
> >> >>>> >> Cheers,
> >> >>>> >> Ida.
> >> >>>> >> _______________________________________________
> >> >>>> >> devel mailing list
> >> >>>> >> devel at rtems.org
> >> >>>> >> http://lists.rtems.org/mailman/listinfo/devel
> >> >>>> > _______________________________________________
> >> >>>> > devel mailing list
> >> >>>> > devel at rtems.org
> >> >>>> > http://lists.rtems.org/mailman/listinfo/devel
> >> >>>> >
> >> >>>> _______________________________________________
> >> >>>> devel mailing list
> >> >>>> devel at rtems.org
> >> >>>> http://lists.rtems.org/mailman/listinfo/devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20210604/0551892f/attachment.html>