Do not use `assert!` for expressions intended to have side effect

[wks]

In Rust, assert! is guaranteed to be executed in the release build, and cannot be disabled. debug_assert!, on the other hand, is only executed in the debug build.

Despite of this, the assert! macro is still intended for assertion, only. An assertion statement checks whether an invariant holds at a given point of execution. From humans' point of view, assertions can be omitted without affecting the correctness of the program. Programmers familiar with other programming languages may also feel that assertions are not enabled in release builds. For this reason, we should use assert! and debug_assert! only for checking the variants. The sub-expression intended to be used for their side effects should not be part of the assert! macro.

For example, the following snippet is found in src/policy/marksweepspace/malloc_ms/metadata.rs

fn map_active_chunk_metadata(chunk_start: Address) {
    // ...
    assert!(
        CHUNK_METADATA.try_map_metadata_space(start, size).is_ok(),
        "failed to mmap meta memory"
    );
}

The code intends to map metadata space for an address region, and expect it to be successful. If not successful, it is either a bug (e.g. the address range overlaps with other spaces) or an unrecoverable error (e.g. the mmap system call failed). Here the call of try_map_metadata_space is part of the normal operation, and the expected result is that it returns Ok(()). So the call of try_map_meta_space should not be part of the assertion. The above code should be rewritten as

    let result = CHUNK_METADATA.try_map_metadata_space(start, size);
    assert!(result.is_ok(), "failed to mmap meta memory");

Assertion tends to mean "if the predicate is not true, it is a bug". But given that Err() may be a result of runtime error, it is better to use explicit panic! rather than assert!. We can rewrite it as:

    CHUNK_METADATA
        .try_map_metadata_space(start, size)
        .unwrap_or_else(|_err| {
            panic!("failed to mmap meta memory");
        });

Or more concisely:

    CHUNK_METADATA
        .try_map_metadata_space(start, size)
        .expect("failed to mmap meta memory");

We should go through the mmtk-core code base to find and fix similar use cases.