Skip to content

Wrong type mismatch with cfg_if! in function body #12940

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

Closed
jonas-schievink opened this issue Aug 4, 2022 · 2 comments · Fixed by #13027
Closed

Wrong type mismatch with cfg_if! in function body #12940

jonas-schievink opened this issue Aug 4, 2022 · 2 comments · Fixed by #13027
Assignees
Labels
A-macro macro expansion A-ty type system / type inference / traits / method resolution C-bug Category: bug S-actionable Someone could pick this issue up and work on it right now

Comments

@jonas-schievink
Copy link
Contributor

This code reports an incorrect "expected MemoryUsage, found ()" type mismatch error:

cfg_if! {
if #[cfg(all(feature = "jemalloc", not(target_env = "msvc")))] {
jemalloc_ctl::epoch::advance().unwrap();
MemoryUsage {
allocated: Bytes(jemalloc_ctl::stats::allocated::read().unwrap() as isize),
}
} else if #[cfg(all(target_os = "linux", target_env = "gnu"))] {
memusage_linux()
} else if #[cfg(windows)] {
// There doesn't seem to be an API for determining heap usage, so we try to
// approximate that by using the Commit Charge value.
use winapi::um::processthreadsapi::*;
use winapi::um::psapi::*;
use std::mem::{MaybeUninit, size_of};
let proc = unsafe { GetCurrentProcess() };
let mut mem_counters = MaybeUninit::uninit();
let cb = size_of::<PROCESS_MEMORY_COUNTERS>();
let ret = unsafe { GetProcessMemoryInfo(proc, mem_counters.as_mut_ptr(), cb as u32) };
assert!(ret != 0);
let usage = unsafe { mem_counters.assume_init().PagefileUsage };
MemoryUsage { allocated: Bytes(usage as isize) }
} else {
MemoryUsage { allocated: Bytes(0) }
}
}

Might be due to incorrect handling of macros in block tail position.

@jonas-schievink jonas-schievink added A-ty type system / type inference / traits / method resolution A-macro macro expansion S-actionable Someone could pick this issue up and work on it right now C-bug Category: bug labels Aug 4, 2022
bors added a commit that referenced this issue Aug 15, 2022
internal: Add an HIR pretty-printer

This improves the "View HIR" command by pretty-printing the HIR to make it much more readable.

Example function:

```rust
    fn newline(&mut self) {
        match self.buf.chars().rev().skip_while(|ch| *ch == ' ').next() {
            Some('\n') | None => {}
            _ => writeln!(self).unwrap(),
        }
    }
```

Previous output:

```
HIR expressions in the body of `newline`:
Idx::<Expr>(0): Path(Path { type_anchor: None, mod_path: ModPath { kind: Super(0), segments: [] }, generic_args: [] })
Idx::<Expr>(1): Field { expr: Idx::<Expr>(0), name: Name(Text("buf")) }
Idx::<Expr>(2): MethodCall { receiver: Idx::<Expr>(1), method_name: Name(Text("chars")), args: [], generic_args: None }
Idx::<Expr>(3): MethodCall { receiver: Idx::<Expr>(2), method_name: Name(Text("rev")), args: [], generic_args: None }
Idx::<Expr>(4): Path(Path { type_anchor: None, mod_path: ModPath { kind: Plain, segments: [Name(Text("ch"))] }, generic_args: [None] })
Idx::<Expr>(5): UnaryOp { expr: Idx::<Expr>(4), op: Deref }
Idx::<Expr>(6): Literal(Char(' '))
Idx::<Expr>(7): BinaryOp { lhs: Idx::<Expr>(5), rhs: Idx::<Expr>(6), op: Some(CmpOp(Eq { negated: false })) }
Idx::<Expr>(8): Closure { args: [Idx::<Pat>(1)], arg_types: [None], ret_type: None, body: Idx::<Expr>(7) }
Idx::<Expr>(9): MethodCall { receiver: Idx::<Expr>(3), method_name: Name(Text("skip_while")), args: [Idx::<Expr>(8)], generic_args: None }
Idx::<Expr>(10): MethodCall { receiver: Idx::<Expr>(9), method_name: Name(Text("next")), args: [], generic_args: None }
Idx::<Expr>(11): Literal(Char('\n'))
Idx::<Expr>(12): Block { id: BlockId(37), statements: [], tail: None, label: None }
Idx::<Expr>(13): Path(Path { type_anchor: None, mod_path: ModPath { kind: Super(0), segments: [] }, generic_args: [] })
Idx::<Expr>(14): Path(Path { type_anchor: None, mod_path: ModPath { kind: Plain, segments: [Name(Text("std")), Name(Text("fmt")), Name(Text("Arguments")), Name(Text("new_v1"))] }, generic_args: [None, None, None, None] })
Idx::<Expr>(15): Array(ElementList { elements: [], is_assignee_expr: false })
Idx::<Expr>(16): Ref { expr: Idx::<Expr>(15), rawness: Ref, mutability: Shared }
Idx::<Expr>(17): Array(ElementList { elements: [], is_assignee_expr: false })
Idx::<Expr>(18): Ref { expr: Idx::<Expr>(17), rawness: Ref, mutability: Shared }
Idx::<Expr>(19): Call { callee: Idx::<Expr>(14), args: [Idx::<Expr>(16), Idx::<Expr>(18)], is_assignee_expr: false }
Idx::<Expr>(20): MethodCall { receiver: Idx::<Expr>(13), method_name: Name(Text("write_fmt")), args: [Idx::<Expr>(19)], generic_args: None }
Idx::<Expr>(21): MacroStmts { statements: [], tail: Some(Idx::<Expr>(20)) }
Idx::<Expr>(22): MethodCall { receiver: Idx::<Expr>(21), method_name: Name(Text("unwrap")), args: [], generic_args: None }
Idx::<Expr>(23): Match { expr: Idx::<Expr>(10), arms: [MatchArm { pat: Idx::<Pat>(5), guard: None, expr: Idx::<Expr>(12) }, MatchArm { pat: Idx::<Pat>(6), guard: None, expr: Idx::<Expr>(22) }] }
Idx::<Expr>(24): Block { id: BlockId(36), statements: [], tail: Some(Idx::<Expr>(23)), label: None }
```

Output after this PR:

```rust
fn newline(…) {
    match self.buf.chars().rev().skip_while(
        |ch| (*ch) == (' '),
    ).next() {
        Some('\n') | None => {},
        _ => { // macro statements
            self.write_fmt(
                std::fmt::Arguments::new_v1(
                    &[],
                    &[],
                ),
            )
        }.unwrap(),
    }
}
```

It also works for consts and statics now.

This should make debugging HIR-lowering related issues like #12940 much easier.
@jonas-schievink
Copy link
Contributor Author

Reduced to:

macro_rules! m2 {
    ($($t:tt)*) => {$($t)*};
}

pub fn macrostmts() -> u8 {
    m2! { 0 }

    m2! {}
}

The HIR looks like this:

fn macrostmts(…) {
    { // macro statements
        0
    }
    { // macro statements
    
    }
}

so it seems that an empty MacroStmts expression is treated like an empty block, which is wrong.

@Veykril
Copy link
Member

Veykril commented Aug 15, 2022

Might've regressed in #12668

@jonas-schievink jonas-schievink changed the title Wrong type mismatch in MemoryUsage::now() Wrong type mismatch with cfg_if! in function body Aug 15, 2022
@jonas-schievink jonas-schievink self-assigned this Aug 15, 2022
bors added a commit that referenced this issue Aug 15, 2022
…ty-macro, r=jonas-schievink

fix: Fix incorrect type mismatch with `cfg_if!` and other macros in expression position

Fixes #12940

This is a bit of a hack, ideally `MacroStmts` would not exist at all after HIR lowering, but that requires changing how the lowering code works.
@bors bors closed this as completed in 3903243 Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macro macro expansion A-ty type system / type inference / traits / method resolution C-bug Category: bug S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants