GSoC - Code Formatting and Style Checking for RTEMS score
Ida Delphine
idadelm at gmail.com
Mon May 31 20:58:37 UTC 2021
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?
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/20210531/c714d6fd/attachment.html>
More information about the devel
mailing list