transpile: Various fixes to type handling for builtins

[Rua]

• Depends on transpile: Ensure that there are always types for primitive type kinds #1865 (to ensure that CTypeKind::Bool is always present)
• Fixes C2Rust emits size_t for __builtin_object_size cast to unsigned long, causing Rust type mismatch #1707

[fw-immunant]

LGTM overall, but would appreciate a comment where mentioned inline.

[fw-immunant]

NAK on the comment change - now the name of bubble_expr_types and its comment disagree on its primary purpose. It exists to homogeneize types between Clang versions by bubbling up types from child AST nodes to parent ones (and these types are not necessarily ones in PULLBACK_KINDS, e.g. in the case of bit shifts), and the other things it does are unrelated to this function. They should really be done separately, so we can remove the function when we no longer support Clang <16. Translating arbitrary C operations producing one type to Rust ones that producing a different type (based on the operation's identity rather than its children) seems best done in expr translation rather than by altering types here.

We don't necessarily need to change where this logic happens in this PR, but I meant to ask for a comment that documents the tech debt here rather than making it sound like the current confusing arrangement is by design.

[Rua]

They should really be done separately, so we can remove the function when we no longer support Clang <16.

Iterating through the whole AST is somewhat expensive, so I figure it's more efficient to fit it all together like this.

Translating arbitrary C operations producing one type to Rust ones that producing a different type (based on the operation's identity rather than its children) seems best done in expr translation rather than by altering types here.

Perhaps, but the difficulty here is that the "bubbling" is done bottom-up while translation operates top-down. For example, if you have an expression like sizeof(Foo) + 1 then you want the override on the sizeof type to propagate to the + expression too. The current code does that, but doing it with a top-down convert_expr call tree is much harder and messier. Though perhaps #1679 could provide a solution there?

[Rua]

I've now renamed bubble_expr_types to better reflect what it actually does.

[ahomescu]

Could use a few more tests for the other builtins.

[Rua]

I don't really know how the builtins are used or what their use cases are, so I wouldn't know what would be a good test for them.

[ahomescu]

See my other comment below for a single line change here.

For other builtins, here's something you could add that should confirm that these builtins return an int that's the same as int32_t:

#include <stdint.h>

void test_builtin_expected_type(double d, int y) {
    int32_t ffs = __builtin_ffs(y);
    int32_t clz = __builtin_clz((unsigned)y);
    int32_t isnan = __builtin_isnan(d);
    int32_t isinf_sign = __builtin_isinf_sign(d);
    int32_t constant_p = __builtin_constant_p(y);
    int32_t cond = y ? __builtin_ffs(y) : 0;
}

[ahomescu]

Now we have a literal suffix and 2 casts? That seems like overkill.

[Rua]

The isize suffix is needed because -1 is negative and won't fit in a usize. The final cast is the one that casts the result of the builtin to ulong, so it mimics the C AST.

[ahomescu]

You could add a second line here to test an explicit cast:

unsigned long n2 = (unsigned long)__builtin_object_size(&x, 1);

[ahomescu]

Two of these use result_type_id, the third target_type_id. Shouldn't they all be the same?