column! should return a fixed size integer

[vks]

Currently column! returns a uint, which does not make sense, because the number of the column in the source code is not related to the pointer size. It should be u32 or u64.

[kmcallister]

This is not backwards-compatible so let's change it before 1.0-final.

[brooksbp]

I'd like to take a shot at this one. Could there be a plausible reason for it being a u64 over a u32?

[Gankra]

u32 should be fine.

[kmcallister]

u32 should be fine. The BytePos in Span is a u32.

[kmcallister]

Did you check line! as well? They should use the same type I'd think.

[brooksbp]

@kmcallister Thanks, I was thinking the same thing too.

[vks]

In theory, very large source code file (> 4 GiB) might require u64. I'm not sure this is relevant in practice though.

[brooksbp]

This quickly turned into an interesting problem..

I modified expand_line(..) in src/libsyntax/ext/source_util.rs to return a ast::UnsignedIntLit(ast::TyU32).

I then modified the few places in src/* that explicitly typed line!() as usize to be u32. However, the stage0 compiler will still expand line!() : usize, so there were type mismatch errors: expected: u32, got usize. So, I just cast the macro calls: line!() as u32. This led to hitting an LLVM assertion in the stage0.. something about a mismatch between function call and its definition wrt argument types or arity. With RUST_LOG=debug RUST_BACKTRACE=1 VERBOSE=1 it was hitting a Rust assertion in trait type checking. Decided this approach was too much of a black hole and tried for something simpler..

I left the types as usize, but cast the macro call: line()! as usize. This is redundant for the stage0 compile, but for subsequent builds we'll get the u32 -> usize. My thinking is that this initial code change is needed in the stage0 before the remainder of the usize -> u32 conversions could be made..

Looking for a thumbs-up or thumbs-down on this approach before continuing.. Thx

[pnkfelix]

(part of #20761)

[brooksbp]

The Rust assertion and backtrace looked very similar to #21682 .

[vojtechkral]

@brooksbp Couldn't this be resolved using a temporary #[cfg(stage0)] directive? (Or rather a bunch thereof...)

[brooksbp]

@vojtechkral @nick29581 With #[cfg(stage0)] I am hitting the LLVM assertion in the stage0 as previous described above.

LLVM assertion in stage0:

rustc: /home/rustbuild/src/rust-buildbot/slave/snap3-linux/build/src/llvm/lib/IR/Instructions.cpp:281: void llvm::CallInst::init(llvm::Value*, llvm::ArrayRef<llvm::Value*>, const llvm::Twine&): Assertion `(i >= FTy->getNumParams() || FTy->getParamType(i) == Args[i]->getType()) && "Calling a function with a bad signature!"' failed.
/home/brian/code/rust/mk/target.mk:165: recipe for target 'x86_64-unknown-linux-gnu/stage0/lib/rustlib/x86_64-unknown-linux-gnu/lib/stamp.core' failed

Backtrace:

(gdb) bt
#0  0x00007f51e5ca44b7 in raise () from /usr/lib/libc.so.6
#1  0x00007f51e5ca588a in abort () from /usr/lib/libc.so.6
#2  0x00007f51e5c9d41d in __assert_fail_base () from /usr/lib/libc.so.6
#3  0x00007f51e5c9d4d2 in __assert_fail () from /usr/lib/libc.so.6
#4  0x00007f51e840e83e in llvm::CallInst::init(llvm::Value*, llvm::ArrayRef<llvm::Value*>, llvm::Twine const&) ()
#5  0x00007f51e78e1f69 in llvm::IRBuilder<true, llvm::ConstantFolder, llvm::IRBuilderDefaultInserter<true> >::CreateCall(llvm::Value*, llvm::ArrayRef<llvm::Value*>, llvm::Twine const&) ()
#6  0x00007f51e839c1c6 in LLVMBuildCall ()
#7  0x00007f51e713f7af in trans::builder::Builder$LT$$u{27}a$C$$u{20}$u{27}tcx$GT$::call::h876586d31c9b3666aEr ()
#8  0x00007f51e7090d58 in trans::build::Call::had877430a1c1759aUoq ()
#9  0x00007f51e709ff40 in trans::base::invoke::he792c282be64deb9c0s ()
#10 0x00007f51e708aba2 in trans::callee::trans_lang_call::hba6b434045fe7e80ORg ()
#11 0x00007f51e7089325 in trans::controlflow::trans_fail::h577b588f6e7318127te ()
#12 0x00007f51e7107a30 in trans::base::fail_if_zero_or_overflows::h2d841423f525d47arTs ()
#13 0x00007f51e7106832 in trans::expr::trans_eager_binop::h773c177151cc0550tMj ()
#14 0x00007f51e70e3dd5 in trans::expr::trans_binary::hdaa99015117cbed4KVj ()
#15 0x00007f51e70ccc1a in trans::expr::trans_unadjusted::h4e139eeb46c210cdQli ()
#16 0x00007f51e70852b8 in trans::expr::trans_into::h225d9e6c2923fafdlAh ()
#17 0x00007f51e7183603 in trans::_match::mk_binding_alloca::h5359920857975042623 ()
#18 0x00007f51e70847ce in trans::base::init_local::hc1689d013dbfe470Zet ()
#19 0x00007f51e7085ad4 in trans::controlflow::trans_block::ha23567704a31fa16xae ()
#20 0x00007f51e715127b in trans::base::trans_closure::h53a2304d5cce6cecX3t ()
#21 0x00007f51e70721d1 in trans::base::trans_fn::he388c9e64e690a1f7eu ()
#22 0x00007f51e706dfca in trans::base::trans_item::hd8e315fbfc388007yDu ()
#23 0x00007f51e7152a69 in trans::base::trans_mod::habbfd992c0433f8fnJu ()
#24 0x00007f51e706d552 in trans::base::trans_item::hd8e315fbfc388007yDu ()
#25 0x00007f51e71572c9 in trans::base::trans_crate::hf665da3716c47588iAv ()
#26 0x00007f51e6f809be in driver::phase_4_translate_to_llvm::h661e1c54716425323Oa ()
#27 0x00007f51e6f5b460 in driver::compile_input::h1ed0a8b4db6185f4Cba ()
#28 0x00007f51e702bc64 in run_compiler::h7e8f027b670e6f197ac ()
#29 0x00007f51e7028fad in thunk::F.Invoke$LT$A$C$$u{20}R$GT$::invoke::h10957786224773965967 ()
#30 0x00007f51e7027d6f in rt::unwind::try::try_fn::h11972162461772211631 ()
#31 0x00007f51e8cf2ce9 in rust_try_inner ()
#32 0x00007f51e8cf2cd6 in rust_try ()
#33 0x00007f51e70283bc in thunk::F.Invoke$LT$A$C$$u{20}R$GT$::invoke::h17437111990391212063 ()
#34 0x00007f51e8ced366 in sys::thread::thread_start::h9f6730fbc95b528aUCB ()
#35 0x00007f51e6942374 in start_thread () from /usr/lib/libpthread.so.0
#36 0x00007f51e5d5927d in clone () from /usr/lib/libc.so.6

The diff I am working with is located in #21769. And here https://gist.github.com/brooksbp/babc8303ce5982311ef4

Any suggestions on how to further debug this would be appreciated. Thanks!

[vojtechkral]

@brooksbp I think it's because you changed the signature of panic_impl, which is an external thing. See http://doc.rust-lang.org/core/panicking/ . I think it's provided by llvm, but I can't seem to find where it's implemented...

EDIT: Nope, it's defined in src/libstd/rt/unwind.rs as rust_begin_unwind()

[brooksbp]

@vojtechkral thanks. Still hitting the LLVM assertion after reverting the type change in the signature of panic_impl and casting the argument instead (line as uint):

pub fn panic_fmt(fmt: fmt::Arguments, file_line: &(&'static str, u32)) -> ! {
    #[allow(improper_ctypes)]
    extern {
        #[lang = "panic_fmt"]
        fn panic_impl(fmt: fmt::Arguments, file: &'static str, line: uint) -> !;
    }
    let (file, line) = *file_line;
    unsafe { panic_impl(fmt, file, line as uint) }
}

[vojtechkral]

@brooksbp Finally found the real cause, it was by an indirect call from src/librustc_trans/trans/controlflow.rs . See the code in commit referenced (for some reason gist wouldn't accept my diff file, so I commited the changes in my fork instead). I think it should be working, my tests didn't quite finish yet, but stages 1 & 2 are compiled and I have to go now...

[brooksbp]

@vojtechkral Thanks. I have reproduced what you're seeing. I think more than the controlflow.rs changes were required as I had to 'lift' some of the #[cfg(..)] stmts higher in scope as you mentioned earlier.

@vojtechkral How did you end up zero'ing in on the code in controlflow.rs? Did you use any extra logging or debug in the stage0?

[vojtechkral]

@brooksbp No, I don't really know how to debug those asserts to more detail. I found out that the #[lang="..."] refers to stuff defined in src/librustc/middle/lang_items.rs and followed the smybols therein, basically just a bunch of grep comands...

[mitaa]

This seems resolved.

[apasel422]

This was indeed fixed and can be closed.