refactor: Fix name collisions in reorganize_definitions

[ahomescu]

Rename identically-named identifier (appending a suffix) for identifiers with different definitions, e.g.

    pub const EINPROGRESS: ::core::ffi::c_int = 115;
    pub const EINPROGRESS: ::core::ffi::c_int = 115 as ::core::ffi::c_int;

from libxml2. The transform now renames the second definition to EINPROGRESS_1 and updates all uses.

This is a copy of #1463 since GitHub closed that one after I reordered the branches.

[kkysen]

Do we want to keep these as separate definitions when their values are the same? Is it better to try to fix this from the transpiler side?

[fw-immunant]

Do we want to keep these as separate definitions when their values are the same? Is it better to try to fix this from the transpiler side?

I don't have a preference on whether we merge definitions like these, but separately it would be nice to improve this on the transpiler side, because its root cause is the same as why our handling of switch cases emits integers rather than uses of consts when cases reference #define'd values. But that fix could be a bit tricky; we'd need some special handling to keep types in agreement everywhere.

[ahomescu]

I didn't touch the transpiler because afaict the two values of EINPROGRESS in the example come in as completely different clang AST nodes. If you want to unify the two implementations, go for it, but that does feel like a more significant refactoring to me. Meanwhile, I think the current refactoring fix is also worthwhile to land (otherwise we might hit the problem in other places as well).

[kkysen]

Maybe there should be a warning about this? Is it at all possible to check if the actual values are the same and merge them in that case? Or maybe run remove redundant casts first.

[ahomescu]

Maybe there should be a warning about this?

I could add a warning, sure.

Is it at all possible to check if the actual values are the same and merge them in that case?

We would basically need to run a compile-time evaluator for that, and that's if the expressions are even constant in the first place.

Or maybe run remove redundant casts first.

Sure, but that only fixes the subset where casts are the problem. What about e.g. comparing 42 with 42 + 0?

[kkysen]

Or maybe run remove redundant casts first.

Sure, but that only fixes the subset where casts are the problem. What about e.g. comparing 42 with 42 + 0?

Yeah, we definitely need the general fix as done here, but I think the majority of these that the transpiler produces will be from casts. So we should do that, too (run remove redundant casts first).

[ahomescu]

I added a warning.