Useless check in `generated_cases.c.h`?

[Eclips4]

In generated_cases.c.h we have a lot(20) of "useless" checks like that:

cpython/Python/generated_cases.c.h

Line 541 in af446bb

if (true) goto pop_2_error;

Does it really need to be here, or should we remove this?
I think, we can just use goto ... instead of if (true) goto ...

Linked PRs

• gh-102028: Improve generate_cases.py #102050

[sobolevn]

Code that generates generated_case.c.h has these lines:

out.write_raw(
  f"{space}if ({cond}) {{ STACK_SHRINK({symbolic}); goto {label}; }}\n"
)

And some similar ones.

cond can be defined as res == NULL in ERROR_IF(res == NULL, error); or as ERROR_IF(true, error)

This is the source of if (true) lines. Current solution seems like the simplest one.

[arhadthedev]

@sobolevn Can the example line be rewritten as follows?

action = f'STACK_SHRINK({symbolic}); goto {label};'
out.write_raw(
  f'{space}if ({cond}) {{ {action} }}\n' if cond else f'{space}{action}\n'
)

[sobolevn]

cond is a string, but yeah - something similar might be applied in theory. However, I am not an expert in these modules and I don't what to give false advice :)

[Eclips4]

I tested it, and we can replace it by this code:

                if symbolic:
                    if cond == 'true':
                        action = f'STACK_SHRINK({symbolic});\n{space}goto {label};'
                        string = f'{space}{action}\n'
                    else:
                        action = f'STACK_SHRINK({symbolic}); goto {label};'
                        string = f'{space}if ({cond}) {{ {action} }}\n'
                    out.write_raw(string)
                else:
                    action = f'goto {label};\n'
                    if cond == 'true':
                        string = f'{space}{action}'
                    else:
                        string = f'{space}if ({cond}) goto {label};\n'
                    out.write_raw(string)

Result: https://proxy-ga.blitzz.co/proxy/123456/gist.github.com/Eclips4/92594f62f6a7d7e6d9f30ba8de95c3da

[Eclips4]

Perhaps, we can simplify this piece of code.
But, with this code, generated_cases.c.h looks cool, and no more if (true) :)

[sobolevn]

(I would prefer to have _format_condition helper function/method instead of if cond == 'true' check)

[Eclips4]

def format_condition(cond: str, label: str, space: str, symbolic: str) -> str:
    if symbolic:
        action = f"STACK_SHRINK({symbolic});"
        if cond == "true":
            action += f"\n{space}goto {label};"
            string = f"{space}{action}\n"
        else:
            action += f" goto {label};"
            string = f"{space}if ({cond}) {{ {action} }}\n"
    else:
        action = f"goto {label};\n"
        if cond == "true":
            string = f"{space}{action}"
        else:
            string = f"{space}if ({cond}) goto {label};\n"
    return string

Someting like that?

[rasputyashka]

@Eclips4 inconsistent use of double and single quotes looks weird to me, and I assume the function name should be _format_condition.

[Eclips4]

Other helper functions declared in global scope without _ in name, we can change their names, if they are not used outside of generate_cases.py
I changed the message, now it's contains only double quotes.

[Eclips4]

If there are no more issues, i will send a PR.

[gvanrossum]

I'm sorry, but this is a waste of time. The C compiler's optimizer will elide the if (true) part, so I see no need to special-case it in the generator.