Help needed with codeformat - prepocessing errors

[Josverl]

Im working on a PR to support metaclasses that needs quite some changes to modbuiltins.c.
the new features are guarded with #ifdefs but I think I must have made a mistake somewhere -and I just can't find it
If I try to format the code codeformat thows an error and truncates the file at line 54

@Josverl ➜ /workspaces/micropython (feat/metaclass) $ ./tools/codeformat.py py/modbuiltins.c 
Traceback (most recent call last):
  File "/workspaces/micropython/./tools/codeformat.py", line 200, in <module>
    main()
  File "/workspaces/micropython/./tools/codeformat.py", line 186, in main
    fixup_c(file)
  File "/workspaces/micropython/./tools/codeformat.py", line 119, in fixup_c
    raise IndentationError(
IndentationError: dedent stack is empty for "endif" at py/modbuiltins.c:54

Note the same error also happens in CI in the Code formatting workflow.

Note that uncrustify processes the file without any error, also the code can be compiled and linked successfully in any permutation of the three added guards.

It has been somewhat frustrating to see my work deleted by a pre-commit hook every time I try to check something in .....
(rescued by stashes and turning off precommit)

So please help me sanity check ; so I can finally check in my code .

Questions:

• is there a #if #else #endif rule or best-practice I am breaking.
• is it expected that codeformat removes code it it veers off the happy-path ? ( I don't think so )
• It looks the issue is in def fixup_c(filename) - but I don't grok its logic yet.
  Possibly @DvdGiessen knows ?

for config and code see : https://gist.github.com/Josverl/505bcfb547f321d4e5c8b15c9cf4472c#file-modbuiltins-c

[DvdGiessen]

It doesn't like this part:

https://gist.github.com/Josverl/505bcfb547f321d4e5c8b15c9cf4472c#file-modbuiltins-c-L49-L54

#if MICROPY_METACLASS || MICROPY_INIT_SUBCLASS
// Keyword arguments may include 'metaclass'
static mp_obj_t mp_builtin___build_class__(size_t n_args, const mp_obj_t *args, mp_map_t *kw_args) {
#else
static mp_obj_t mp_builtin___build_class__(size_t n_args, const mp_obj_t *args) {
#endif

Specifically, what it doesn't like is that two indenting rules clash: It tries to keep the #if, #else and #endif on the same indentation level. But at the same time, the #else and #endif are inside the block and should have the same indentation as the rest of the block.

The way to write it instead would be:

#if MICROPY_METACLASS || MICROPY_INIT_SUBCLASS
// Keyword arguments may include 'metaclass'
static mp_obj_t mp_builtin___build_class__(size_t n_args, const mp_obj_t *args, mp_map_t *kw_args)
#else
static mp_obj_t mp_builtin___build_class__(size_t n_args, const mp_obj_t *args)
#endif
{

Aka moving the curly brace outside of the preprocessor block.

EDIT:

I came across it before and then improve the error message, which previously was even less helpful, in #17837.

is it expected that codeformat removes code it it veers off the happy-path ? ( I don't think so )

That's indeed not great. In addition to more helpful errors, there should probably be a try-catch around the entire logic, and if any exception is raised it should revert the file to it's original state. Or, since at that point we're caching the entire file in memory anyway, simpler would be to write the reformatted code to a variable first and only write it back to the filesystem when everything completed without errors.

[Josverl]

@DvdGiessen,
Bedankt for the quick response and the solution.
I thought I tried that already , as adding whitespace - but apparently not with the correct incantation.

With the line number and the #endif I kept looking for mismatched preprocessor statements. So happy with the error message.

Should I create an issue for the truncate error ?