Apply uncrustify to format all code (v1)

[dpgeorge]

Per discussion in #4223 here's an attempt to format the code using the uncrustify tool; see https://github.com/uncrustify/uncrustify

Uncrustify is highly configurable, and seems to support indenting preprocessor directives quite well. It can also add braces {} for single-line if-else statements (which is enabled in the settings here).

It's not perfect but is pretty good. There's still a bit of room left to optimise the settings.

[jimmo]

This looks amazing.

Minor preference to use inline comments (the equivalent of // clang-format off) to enable/disable rather than the explicit exclude list in codeformat.py, but other than that, this is really good.

I think there are a few things set to ignore that would be good to later set to something, but it's also nice to be able to do that incrementally in a future PR. And the fact that it has an ignore mode that works so well is a good argument in favour of uncrustify.

[stinos]

Pretty good yes. I've used uncrustify in the past and mainly remember it took like an hour to get through all configuration :)
Things like struct tcp_pcb*volatile *slot aren't too consistent/readable though. It's good that it adds braces and puts statements on their own line. But I think what clang-format does is even better there (e.g. look at the switch in modujson.c: astyle does nothing, uncrustify puts just the break on a new line and clang-format puts everything on it's own line).

Wrt codeformat.py: I think in the end it needs to be changed to not require the current working directory to be correct i.e. use full absoluta paths instead of relative so that it's easier to invoke from anywhere. And if arguments are passed they should be treated as paths to files to format (also checked agains list of files to exclude, if that's kept). That makes it easier to integrate in text editors etc so you can just format the file you're currently working on.

[dpgeorge]

Things like struct tcp_pcb*volatile *slot aren't too consistent/readable though.

Yes that needs to be fixed.

It's good that it adds braces and puts statements on their own line. But I think what clang-format does is even better there (e.g. look at the switch in modujson.c: astyle does nothing, uncrustify puts just the break on a new line and clang-format puts everything on it's own line).

That also needs to be fixed, to put everything on its own line. Uncrustify respects a lot of existing format style so it may be that the code needs to be fixed by hand to put things on one line, then uncrustify will keep it that way.

Wrt codeformat.py: I think in the end it needs to be changed to not require the current working directory to be correct i.e. use full absoluta paths instead of relative so that it's easier to invoke from anywhere. And if arguments are passed they should be treated as paths to files to format

Agreed (and this goes regardless of which formatter we pick).

[dpgeorge]

@tannewt what do you think about using uncrustify as a C formatter? Big benefit is that it can handle indenting #if's as we currently use them. And I don't see any show-stoppers to using it.

[tannewt]

Looks good to me!

[tannewt]

We can always refine it later or move to clang-format/clang-tidy if there is something we want later.

[jimmo]

Things like struct tcp_pcb*volatile *slot aren't too consistent/readable though.

Yes that needs to be fixed.

There doesn't appear to be a way to get this exactly right, but

sp_after_ptr_star_qualifier = add

help. You get type* qualifier *name.

If we consider changing the casting format to (int *)x rather than (int*)x in the future, then adding sp_before_unnamed_ptr_star = add later will give us type * qualifier *name.

It's good that it adds braces and puts statements on their own line. But I think what clang-format does is even better there (e.g. look at the switch in modujson.c: astyle does nothing, uncrustify puts just the break on a new line and clang-format puts everything on it's own line).

That also needs to be fixed, to put everything on its own line. Uncrustify respects a lot of existing format style so it may be that the code needs to be fixed by hand to put things on one line, then uncrustify will keep it that way.

nl_after_case = true

is what you need here.

Wrt codeformat.py: I think in the end it needs to be changed to not require the current working directory to be correct i.e. use full absoluta paths instead of relative so that it's easier to invoke from anywhere. And if arguments are passed they should be treated as paths to files to format

Agreed (and this goes regardless of which formatter we pick).

[dpgeorge]

Ok, I force-pushed this with some fixes and changes. INDENT-OFF is now used to turn off formatting in specific places, and newlines are always added after the case label in a switch.

Some outstanding items:

• not all C code is reformatted, eg drivers/, extmod/*/, mpy-cross/ and anything deeper than one level in all ports (eg ports/nrf/drivers); maybe these can be left for later (except mpy-cross, that should be reformatted now)
• some inconsistencies in py/obj.h, eg:

 typedef uint64_t mp_obj_t;
 typedef uint64_t mp_const_obj_t;
 #else
-typedef void *mp_obj_t;
-typedef const void *mp_const_obj_t;
+typedef void*mp_obj_t;
+typedef const void*mp_const_obj_t;
 #endif

• also in py/obj.h:

 struct _mp_obj_base_t {
-    const mp_obj_type_t *type MICROPY_OBJ_BASE_ALIGNMENT;
+    const mp_obj_type_t*type MICROPY_OBJ_BASE_ALIGNMENT;
 };
 typedef struct _mp_obj_base_t mp_obj_base_t;

• in py/objfun.c:

 #define DECODE_CODESTATE_SIZE(bytecode, n_state_out_var, state_size_out_var) \
     { \
-        const uint8_t *ip = bytecode; \
+        const uint8_t*ip = bytecode; \
         size_t n_exc_stack, scope_flags, n_pos_args, n_kwonly_args, n_def_args; \
         MP_BC_PRELUDE_SIG_DECODE_INTO(ip, n_state_out_var, n_exc_stack, scope_flags, n_pos_ar
                                     \
         /* state size in bytes */                                                 \
-        state_size_out_var = n_state_out_var * sizeof(mp_obj_t)                   \
-                           + n_exc_stack * sizeof(mp_exc_stack_t);                \
+        state_size_out_var = n_state_out_var*sizeof(mp_obj_t)                   \
+            + n_exc_stack*sizeof(mp_exc_stack_t);                \
     }

[dpgeorge]

Force pushed changes to include mpy-cross in the formatting, and fix the above issues (by changing all casts to have a space before the pointer star).

[dpgeorge]

Closed in favour of #5700.