Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplifying long if-nested-else chains fails #10183

Open
Dante-Broggi opened this issue Jan 9, 2023 · 1 comment
Open

Simplifying long if-nested-else chains fails #10183

Dante-Broggi opened this issue Jan 9, 2023 · 1 comment
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@Dante-Broggi
Copy link

Summary

When running clippy on long if-nested-else chains, like those produced by c2rust in immunant/c2rust#769, though not the exact code shown in that report, clippy warns with a recommendation to report an issue.

Clippy does make progress, and repeating cargo clippy --fix does result in fixed code.

Reproducer

I tried this code:

cargo clippy --fix

On this lib.rs:

#[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
                    } else {
                        if name == (*c).str_I128 {
                            return b"__int128_t\0" as *const u8 as *const libc::c_char
                        } else {
                            if name == (*c).str_ILong {
                                return b"long\0" as *const u8 as *const libc::c_char
                            } else {
                                if name == (*c).str_ISize {
                                    return b"ssize_t\0" as *const u8 as *const libc::c_char
                                } else {
                                    if name == (*c).str_U8 {
                                        return b"char\0" as *const u8 as *const libc::c_char
                                    } else {
                                        if name == (*c).str_U16 {
                                            return b"uint16_t\0" as *const u8 as *const libc::c_char
                                        } else {
                                            if name == (*c).str_U32 {
                                                return b"uint32_t\0" as *const u8 as *const libc::c_char
                                            } else {
                                                if name == (*c).str_U64 {
                                                    return b"uint64_t\0" as *const u8 as *const libc::c_char
                                                } else {
                                                    if name == (*c).str_U128 {
                                                        return b"__uint128_t\0" as *const u8 as *const libc::c_char
                                                    } else {
                                                        if name == (*c).str_ULong {
                                                            return b"unsigned long\0" as *const u8
                                                                as *const libc::c_char
                                                        } else {
                                                            if name == (*c).str_USize {
                                                                return b"size_t\0" as *const u8 as *const libc::c_char
                                                            } else {
                                                                if name == (*c).str_F32 {
                                                                    return b"float\0" as *const u8 as *const libc::c_char
                                                                } else {
                                                                    if name == (*c).str_F64 {
                                                                        return b"double\0" as *const u8 as *const libc::c_char;
                                                                    }
                                                                }
                                                            }
                                                        }
                                                    }
                                                }
                                            }
                                        }
                                    }
                                }
                            }
                        }
                    }
                }
            }
        }
    }
    return 0 as *const libc::c_char;
}
#[derive(Copy, Clone)]
#[repr(C)]
pub struct compile_t {
    pub filename: *const libc::c_char,
    pub str_builtin: *const libc::c_char,
    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,
    pub str_I128: *const libc::c_char,
    pub str_ILong: *const libc::c_char,
    pub str_ISize: *const libc::c_char,
    pub str_U8: *const libc::c_char,
    pub str_U16: *const libc::c_char,
    pub str_U32: *const libc::c_char,
    pub str_U64: *const libc::c_char,
    pub str_U128: *const libc::c_char,
    pub str_ULong: *const libc::c_char,
    pub str_USize: *const libc::c_char,
    pub str_F32: *const libc::c_char,
    pub str_F64: *const libc::c_char,
    pub str_Pointer: *const libc::c_char,
    pub str_NullablePointer: *const libc::c_char,
    pub str_DoNotOptimise: *const libc::c_char,
    pub str_Array: *const libc::c_char,
    pub str_String: *const libc::c_char,
    pub str_Platform: *const libc::c_char,
    pub str_Main: *const libc::c_char,
    pub str_Env: *const libc::c_char,
    pub str_add: *const libc::c_char,
    pub str_sub: *const libc::c_char,
    pub str_mul: *const libc::c_char,
    pub str_div: *const libc::c_char,
    pub str_rem: *const libc::c_char,
    pub str_neg: *const libc::c_char,
    pub str_add_unsafe: *const libc::c_char,
    pub str_sub_unsafe: *const libc::c_char,
    pub str_mul_unsafe: *const libc::c_char,
    pub str_div_unsafe: *const libc::c_char,
    pub str_rem_unsafe: *const libc::c_char,
    pub str_neg_unsafe: *const libc::c_char,
    pub str_and: *const libc::c_char,
    pub str_or: *const libc::c_char,
    pub str_xor: *const libc::c_char,
    pub str_not: *const libc::c_char,
    pub str_shl: *const libc::c_char,
    pub str_shr: *const libc::c_char,
    pub str_shl_unsafe: *const libc::c_char,
    pub str_shr_unsafe: *const libc::c_char,
    pub str_eq: *const libc::c_char,
    pub str_ne: *const libc::c_char,
    pub str_lt: *const libc::c_char,
    pub str_le: *const libc::c_char,
    pub str_ge: *const libc::c_char,
    pub str_gt: *const libc::c_char,
    pub str_eq_unsafe: *const libc::c_char,
    pub str_ne_unsafe: *const libc::c_char,
    pub str_lt_unsafe: *const libc::c_char,
    pub str_le_unsafe: *const libc::c_char,
    pub str_ge_unsafe: *const libc::c_char,
    pub str_gt_unsafe: *const libc::c_char,
    pub str_this: *const libc::c_char,
    pub str_create: *const libc::c_char,
    pub str__create: *const libc::c_char,
    pub str__init: *const libc::c_char,
    pub str__final: *const libc::c_char,
    pub str__event_notify: *const libc::c_char,
    pub str__serialise_space: *const libc::c_char,
    pub str__serialise: *const libc::c_char,
    pub str__deserialise: *const libc::c_char,
}

I expected to see this happen:
clippy writing a fully simplified else-if chain.

Instead, this happened:

    Checking repo v0.1.0 (/private/var/folders/bx/x6mc5wkd69sg5_gsd07xsvc00000gn/T/tmp.pICq7eEq/repo)
warning: error applying suggestions to `src/lib.rs`

The full error message was:

> Could not replace range 5410...6712 in file -- maybe parts of it were already replaced?

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see 
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

warning: error applying suggestions to `src/lib.rs`

The full error message was:

> Could not replace range 5170...6758 in file -- maybe parts of it were already replaced?

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see 
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

warning: error applying suggestions to `src/lib.rs`

The full error message was:

> Could not replace range 4942...6800 in file -- maybe parts of it were already replaced?

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see 
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

warning: error applying suggestions to `src/lib.rs`

The full error message was:

> Could not replace range 4726...6838 in file -- maybe parts of it were already replaced?

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see 
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

warning: error applying suggestions to `src/lib.rs`

The full error message was:

> Could not replace range 4527...6872 in file -- maybe parts of it were already replaced?

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see 
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

warning: error applying suggestions to `src/lib.rs`

The full error message was:

> Could not replace range 4334...6902 in file -- maybe parts of it were already replaced?

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see 
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

warning: error applying suggestions to `src/lib.rs`

The full error message was:

> Could not replace range 4156...6928 in file -- maybe parts of it were already replaced?

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see 
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

warning: error applying suggestions to `src/lib.rs`

The full error message was:

> Could not replace range 3985...6950 in file -- maybe parts of it were already replaced?

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see 
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

warning: error applying suggestions to `src/lib.rs`

The full error message was:

> Could not replace range 3830...6968 in file -- maybe parts of it were already replaced?

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see 
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

warning: error applying suggestions to `src/lib.rs`

The full error message was:

> Could not replace range 3687...6982 in file -- maybe parts of it were already replaced?

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see 
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

warning: error applying suggestions to `src/lib.rs`

The full error message was:

> Could not replace range 3556...6992 in file -- maybe parts of it were already replaced?

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see 
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

warning: error applying suggestions to `src/lib.rs`

The full error message was:

> Could not replace range 3439...6998 in file -- maybe parts of it were already replaced?

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see 
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/rust/issues
quoting the full output of this command we'd be very appreciative!
Note that you may be able to make some more progress in the near-term
fixing code with the `--broken-code` flag

       Fixed src/lib.rs (6 fixes)
warning: this `else { if .. }` block can be collapsed
   --> src/lib.rs:97:12
    |
97  |       } else {
    |  ____________^
98  | |         if name == (*c).str_I8 {
99  | |             return b"int8_t\0" as *const u8 as *const libc::c_char
100 | |         } else {
...   |
153 | |         }
154 | |     }
    | |_____^
    |
    = note: `#[warn(clippy::collapsible_else_if)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_else_if
help: collapse nested if block
    |
97  ~     } else if name == (*c).str_I8 {
98  +         return b"int8_t\0" as *const u8 as *const libc::c_char
99  +     } else {
100 +         if name == (*c).str_I16 {
101 +             return b"int16_t\0" as *const u8 as *const libc::c_char
102 +         } else {
103 +             if name == (*c).str_I32 {
104 +                 return b"int32_t\0" as *const u8 as *const libc::c_char
105 +             } else {
106 +                 if name == (*c).str_I64 {
107 +                     return b"int64_t\0" as *const u8 as *const libc::c_char
108 +                 } else {
109 +                     if name == (*c).str_I128 {
110 +                         return b"__int128_t\0" as *const u8 as *const libc::c_char
111 +                     } else {
112 +                         if name == (*c).str_ILong {
113 +                             return b"long\0" as *const u8 as *const libc::c_char
114 +                         } else {
115 +                             if name == (*c).str_ISize {
116 +                                 return b"ssize_t\0" as *const u8 as *const libc::c_char
117 +                             } else {
118 +                                 if name == (*c).str_U8 {
119 +                                     return b"char\0" as *const u8 as *const libc::c_char
120 +                                 } else {
121 +                                     if name == (*c).str_U16 {
122 +                                         return b"uint16_t\0" as *const u8 as *const libc::c_char
123 +                                     } else {
124 +                                         if name == (*c).str_U32 {
125 +                                             return b"uint32_t\0" as *const u8 as *const libc::c_char
126 +                                         } else {
127 +                                             if name == (*c).str_U64 {
128 +                                                 return b"uint64_t\0" as *const u8 as *const libc::c_char
129 +                                             } else {
130 +                                                 if name == (*c).str_U128 {
131 +                                                     return b"__uint128_t\0" as *const u8 as *const libc::c_char
132 +                                                 } else if name == (*c).str_ULong {
133 +                                                     return b"unsigned long\0" as *const u8
134 +                                                         as *const libc::c_char
135 +                                                 } else if name == (*c).str_USize {
136 +                                                     return b"size_t\0" as *const u8 as *const libc::c_char
137 +                                                 } else if name == (*c).str_F32 {
138 +                                                     return b"float\0" as *const u8 as *const libc::c_char
139 +                                                 } else if name == (*c).str_F64 {
140 +                                                     return b"double\0" as *const u8 as *const libc::c_char;
141 +                                                 }
142 +                                             }
143 +                                         }
144 +                                     }
145 +                                 }
146 +                             }
147 +                         }
148 +                     }
149 +                 }
150 +             }
151 +         }
152 +     }
    |

[… Repeated several times …]

warning: unsafe function's docs miss `# Safety` section
   --> src/lib.rs:91:1
    |
91  | / pub unsafe extern "C" fn c_type_name(
92  | |     mut c: *mut compile_t,
93  | |     mut name: *const libc::c_char,
94  | | ) -> *const libc::c_char {
...   |
147 | |     std::ptr::null::<libc::c_char>()
148 | | }
    | |_^
    |
    = note: `#[warn(clippy::missing_safety_doc)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#missing_safety_doc

warning: `repo` (lib test) generated 9 warnings
    Finished dev [unoptimized + debuginfo] target(s) in 2.47s

Version

rustc 1.65.0 (897e37553 2022-11-02)
binary: rustc
commit-hash: 897e37553bba8b42751c67658967889d11ecd120
commit-date: 2022-11-02
host: x86_64-apple-darwin
release: 1.65.0
LLVM version: 15.0.0

Additional Labels

No response

@Dante-Broggi Dante-Broggi added the C-bug Category: Clippy is not doing the correct thing label Jan 9, 2023
@Jarcho
Copy link
Contributor

Jarcho commented Jan 11, 2023

This is kind of a rustfix issue. When multiple suggestions have overlapping spans like in the example, rustfix can only apply one of them. It will then re-check the code, get another set of overlapping spans, and apply one more. This will keep going until either there are no more suggestions, or the iteration limit is reached. Since the iteration limit is low, that's hit before all the if expressions are unnested.

Either clippy can be changed to unnest multiple nested ifs, or rustfix can have the option to run more iterations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing
Projects
None yet
Development

No branches or pull requests

2 participants