Incremental compilation doesn't update static strings.

[pinkisemils]

When changing a static string in a way that doesn't change it's length, cargo build won't rebuild the changed string.
For instance, if the binary has already been built, changing hello to xxxxx will result in a binary that still outputs hello.

static HELLO: &str = "hello";

fn main() {
    println!("{}", HELLO);
}

If built with CARGO_INCREMENTAL=0, everything works as expected. If the length of the string is changed, everything also works as expected.
Tested on rustc 1.26.0-nightly (75af15ee6 2018-03-20).

[michaelwoerister]

Thanks for the bug report, @pinkisemils!

[michaelwoerister]

OK, I can reproduce on nightly. Cannot reproduce on beta though. Could this be MIRI related, @oli-obk? Are we hashing just an ID instead of contents somewhere?

[oli-obk]

Hashing for statics just hashes the DefId, yes.

rust/src/librustc/ich/impls_ty.rs

Lines 392 to 416 in 15add36

	impl<'a> HashStable<StableHashingContext<'a>> for mir::interpret::AllocId {
	fn hash_stable<W: StableHasherResult>(
	&self,
	hcx: &mut StableHashingContext<'a>,
	hasher: &mut StableHasher<W>,
	) {
	ty::tls::with_opt(|tcx| {
	let tcx = tcx.expect("can't hash AllocIds during hir lowering");
	if let Some(def_id) = tcx.interpret_interner.get_corresponding_static_def_id(*self) {
	AllocDiscriminant::Static.hash_stable(hcx, hasher);
	// statics are unique via their DefId
	def_id.hash_stable(hcx, hasher);
	} else if let Some(alloc) = tcx.interpret_interner.get_alloc(*self) {
	// not a static, can't be recursive, hash the allocation
	AllocDiscriminant::Constant.hash_stable(hcx, hasher);
	alloc.hash_stable(hcx, hasher);
	} else if let Some(inst) = tcx.interpret_interner.get_fn(*self) {
	AllocDiscriminant::Function.hash_stable(hcx, hasher);
	inst.hash_stable(hcx, hasher);
	} else {
	bug!("no allocation for {}", self);
	}
	});
	}
	}

[oli-obk]

So... we can't use recursion, because of recursive statics (which is the entire reason for the DefId-hashing), but we could keep a Vec of the current recursion stack and hash a 0 instead of recursing into an already seen memory and a 1 before every other "recursion" (hashing of an alloc that we're referring to).

This means we need to manually hash the allocation instead of using a generated impl.

[michaelwoerister]

@oli-obk sounds good. I think you don't even need to hash the 0s and 1s as disambiguators, simply not recursing into allocations that have already been visited should be enough. Also, I would just use a already_visited hash set for keeping tracking of where to recurse. That should scale better than a stack that would require linear search.
This additional state can be kept in the StableHashingContext. That's what it's there for.

[oli-obk]

Oh so we could recurse! That makes things easier. I'll try to get to it on Sunday, but we might still end up needing to backport to beta.

[michaelwoerister]

Probably, yes. But the fix should be small and self-contained, so that's fine.

[oli-obk]

Fixed in #49424