pacak / cargo-show-asm

cargo subcommand showing the assembly, LLVM-IR and MIR generated for Rust code
Apache License 2.0
714 stars 35 forks source link

Fix item selection if there is only one item found #312

Closed zheland closed 1 month ago

zheland commented 1 month ago

When an item is specified in arguments, it is expected that no matter how many items are found, the requested item will be shown. When the --everything option is given, it is expected that no matter how many items are found, "everything" will be shown.

Currently (without this PR) if there is only one item found, it will always return only this item, regardless of what is requested on the command line.

This PR moves the automatic selection of a single item later, after checking that we have not selected anything (or --everything) for display.

Also, if you don't mind, this PR removes the unnecessary expect and unreachable in the moved fragment with a little refactoring. I can revert some of the changes if something is not okay or less readable.

Example

pub static DATA: &[u8] = &[1, 2, 3, 4];
#[inline(never)]
pub fn ret_true() -> bool {
    true
}

Current output (without this PR)

$ cargo asm --lib ret_true
.section .text.example::ret_true,"ax",@progbits
    .globl  example::ret_true
    .p2align    4, 0x90
    .type   example::ret_true,@function
example::ret_true:
    .cfi_startproc
    mov al, 1
    ret

$ cargo asm --lib ret_false
.section .text.example::ret_true,"ax",@progbits
    .globl  example::ret_true
    .p2align    4, 0x90
    .type   example::ret_true,@function
example::ret_true:
    .cfi_startproc
    mov al, 1
    ret

$ echo $?
0

$ cargo asm --lib --everything
.section .text.example::ret_true,"ax",@progbits
    .globl  example::ret_true
    .p2align    4, 0x90
    .type   example::ret_true,@function
example::ret_true:
    .cfi_startproc
    mov al, 1
    ret

Expected (with this PR)

$ cargo asm --lib ret_true
.section .text.example::ret_true,"ax",@progbits
    .globl  example::ret_true
    .p2align    4, 0x90
    .type   example::ret_true,@function
example::ret_true:
    .cfi_startproc
    mov al, 1
    ret

$ cargo asm --lib ret_false
Can't find any items matching "ret_false"

$ echo $?
1

$ cargo asm --lib --everything
    .text
    .intel_syntax noprefix
    .file   "example.cb5693eae6d1f11e-cgu.0"
.section .text.example::ret_true,"ax",@progbits
    .globl  example::ret_true
    .p2align    4, 0x90
    .type   example::ret_true,@function
example::ret_true:
    .cfi_startproc
    mov al, 1
    ret
    .size   example::ret_true, .Lfunc_end0-example::ret_true
    .cfi_endproc
    .type   .L__unnamed_1,@object
.section .rodata.cst4,"aM",@progbits,4
    .ascii  "\001\002\003\004"
    .size   .L__unnamed_1, 4
    .type   example::DATA,@object
.section .data.rel.ro.example::DATA,"aw",@progbits
    .globl  example::DATA
    .p2align    3, 0x0
example::DATA:
    .quad   .L__unnamed_1
    .asciz  "\004\000\000\000\000\000\000"
    .size   example::DATA, 16
.section .debug_abbrev,"",@progbits
................................................................

Tests

Checked with example about on linux x86_64 and with cargo test.

pacak commented 1 month ago

Looks great.

Also, if you don't mind, this PR removes the unnecessary expect and unreachable in the moved fragment with a little refactoring.

Loving it :) Can you move it in a separate commit though? First commit - this fix, second commit - smarter selection fix.

zheland commented 1 month ago

Sorry for the strange intermediate push to PR, I don't know how that happened, re-pushed (fixed).

I hope I correctly understood the commit order you meant. First "refactor", second "bug" fix?

pacak commented 1 month ago

Sorry for the strange intermediate push to PR, I don't know how that happened, re-pushed (fixed).

No worries, git does strange things sometimes :)

I hope I correctly understood the commit order you meant.

I imagined the first commit will have only -> ! and -unreachable!("suggest_name exits"); parts, second one - everything else, but what you did seems more correct. CI passes, I'll try to merge/release later today.