c2rust handles else-if chains poorly

[Dante-Broggi]

c2rust handles else-if chains poorly, see example.

As an example this function:

typedef struct compile_t
{

  const char* str_Bool;
  const char* str_I8;
  const char* str_I16;
  const char* str_I32;
  const char* str_I64;
} compile_t;

const char* c_type_name(compile_t* c, const char* name)
{
  if(name == c->str_Bool)
    return "bool";
  else if(name == c->str_I8)
    return "int8_t";
  else if(name == c->str_I16)
    return "int16_t";
  else if(name == c->str_I32)
    return "int32_t";
  else if(name == c->str_I64)
    return "int64_t";

  return 0;
}

Currently translates on the website (https://c2rust.com) as:

#[derive(Copy, Clone)]
#[repr(C)]
pub struct compile_t {
    pub str_Bool: *const libc::c_char,
    pub str_I8: *const libc::c_char,
    pub str_I16: *const libc::c_char,
    pub str_I32: *const libc::c_char,
    pub str_I64: *const libc::c_char,
}
#[no_mangle]
pub unsafe extern "C" fn c_type_name(
    mut c: *mut compile_t,
    mut name: *const libc::c_char,
) -> *const libc::c_char {
    if name == (*c).str_Bool {
        return b"bool\0" as *const u8 as *const libc::c_char
    } else {
        if name == (*c).str_I8 {
            return b"int8_t\0" as *const u8 as *const libc::c_char
        } else {
            if name == (*c).str_I16 {
                return b"int16_t\0" as *const u8 as *const libc::c_char
            } else {
                if name == (*c).str_I32 {
                    return b"int32_t\0" as *const u8 as *const libc::c_char
                } else {
                    if name == (*c).str_I64 {
                        return b"int64_t\0" as *const u8 as *const libc::c_char;
                    }
                }
            }
        }
    }
    return 0 as *const libc::c_char;
}

(The nesting depth gets worse as the number of else-ifs increases.)
But it could translate as:

#[derive(Copy, Clone)]
#[repr(C)]
pub struct compile_t {
    pub str_Bool: *const libc::c_char,
    pub str_I8: *const libc::c_char,
    pub str_I16: *const libc::c_char,
    pub str_I32: *const libc::c_char,
    pub str_I64: *const libc::c_char,
}
#[no_mangle]
pub unsafe extern "C" fn c_type_name(
    mut c: *mut compile_t,
    mut name: *const libc::c_char,
) -> *const libc::c_char {
    if name == (*c).str_Bool {
        return b"bool\0" as *const u8 as *const libc::c_char
    } else if name == (*c).str_I8 {
        return b"int8_t\0" as *const u8 as *const libc::c_char
    } else if name == (*c).str_I16 {
        return b"int16_t\0" as *const u8 as *const libc::c_char
    } else if name == (*c).str_I32 {
        return b"int32_t\0" as *const u8 as *const libc::c_char
    } else if name == (*c).str_I64 {
        return b"int64_t\0" as *const u8 as *const libc::c_char;
    }
    return 0 as *const libc::c_char;
}

This translation would be better formatted, and better preserve the format of the original code.

[kkysen]

Is this something clippy warns on or can fix at all?

[Dante-Broggi]

Clippy works on the small example here, but warns on and takes multiple iterations to fix longer examples.
See the clippy bug report.

[kkysen]

I do think clippy should handle this case, so thanks for the bug report. That said, c2rust transpile can definitely generate better code here for a very common construct. We have more important priorities right now, however (lifting unsafe Rust to safe Rust, as this fix would be for readability, not correctness), so we probably won't be able to dedicate time to fix this anytime soon, though I agree we eventually should. If you have a PR to fix this, though, we'd definitely welcome that!

[bungcip]

For some information, this simple c code

int simple(int i){
    if(i == 0){
        return 0;
    }else if(i == 10){
        return 10;
    }else if(i == 20){
        return 20;
    }else if(i == 30){
        return 30;
    }else{
        return -1;
    }
}

actually producing correct else if chain (running in c2rust 0.17)

#![allow(dead_code, mutable_transmutes, non_camel_case_types, non_snake_case, non_upper_case_globals, unused_assignments, unused_mut)]
#[no_mangle]
pub unsafe extern "C" fn simple(mut i: libc::c_int) -> libc::c_int {
    if i == 0 as libc::c_int {
        return 0 as libc::c_int
    } else if i == 10 as libc::c_int {
        return 10 as libc::c_int
    } else if i == 20 as libc::c_int {
        return 20 as libc::c_int
    } else if i == 30 as libc::c_int {
        return 30 as libc::c_int
    } else {
        return -(1 as libc::c_int)
    };
}

[bungcip]

however, if last else-if-chain dont have else like this:

int simple(int i){
    if(i == 0){
        return 0;
    }else if(i == 10){
        return 10;
    }else if(i == 20){
        return 20;
    }else if(i == 30){
        return 30;
    }
    
    return -1;
}

it will produce:

#[no_mangle]
pub unsafe extern "C" fn simple(mut i: libc::c_int) -> libc::c_int {
    if i == 0 as libc::c_int {
        return 0 as libc::c_int
    } else {
        if i == 10 as libc::c_int {
            return 10 as libc::c_int
        } else {
            if i == 20 as libc::c_int {
                return 20 as libc::c_int
            } else {
                if i == 30 as libc::c_int {
                    return 30 as libc::c_int;
                }
            }
        }
    }
    return -(1 as libc::c_int);
}