supposedly unused unsafe blocks are really unsafe

[metajack]

With #5797 many warnings were generated in Servo code. I attempted to remove these unused unsafe blocks, but I am very concerned that this lint mode is inaccurate. See metajack/servo@868c740#commitcomment-3066562 for a commit that has some comments from @jdm as well.

[metajack]

ping @alexcrichton

[alexcrichton]

Was that second reference to 5797 a typo? I don't see any comments by jdm there...

[metajack]

Yes. I've updated the description.

[alexcrichton]

Does servo not build with the unsafe blocks removed? If it builds then it's not a problem with the warning...

[metajack]

It does build, yes. Perhaps the warning has just uncovered a compiler bug?

[alexcrichton]

Well, this is either a compiler bug, or those functions legitimately don't need to be unsafe, I'd recommend double-checking that the definition of those functions in question are indeed marked as unsafe fn

[metajack]

The first thing in that commit is calling an extern fn FT_Get_Sfnt_Table(face: FT_Face, tag: FT_Sfnt_Tag) -> *c_void;. Isn't this the definition of unsafe?

Farther down by one of @jdm's comments is a call to this extern:

pub fn JS_SetReservedSlot(++obj: JSRawObject, ++index: uint32_t, ++v: JSVal);

JSRawObject is an unsafe pointer here, but that shouldn't even matter since this is an FFI call.

[metajack]

@brson @pcwalton comments?

[pcwalton]

Yes, all C functions are unsafe (if that's what's being asked).

[brson]

@metajack I think it's a bug. I also saw some suspiciously unsafe code in passing recently but didn't think much of it at the time.

[thestinger]

A shot in the dark is that the pub use statements break this.

[alexcrichton]

I believe this was fixed when @pcwalton introduced effect analysis awhile back in aeda178.