Skip to content

Make RustString an extern type to avoid improper_ctypes warnings #132549

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

Merged
merged 2 commits into from
Nov 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions compiler/rustc_codegen_llvm/src/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1766,11 +1766,9 @@ unsafe extern "C" {
pub fn LLVMRustGetLastError() -> *const c_char;

/// Prints the timing information collected by `-Ztime-llvm-passes`.
#[expect(improper_ctypes)]
pub(crate) fn LLVMRustPrintPassTimings(OutStr: &RustString);

/// Prints the statistics collected by `-Zprint-codegen-stats`.
#[expect(improper_ctypes)]
pub(crate) fn LLVMRustPrintStatistics(OutStr: &RustString);

/// Prepares inline assembly.
Expand All @@ -1791,7 +1789,6 @@ unsafe extern "C" {
ConstraintsLen: size_t,
) -> bool;

#[allow(improper_ctypes)]
pub(crate) fn LLVMRustCoverageWriteFilenamesToBuffer(
Filenames: *const *const c_char,
FilenamesLen: size_t,
Expand All @@ -1800,7 +1797,6 @@ unsafe extern "C" {
BufferOut: &RustString,
);

#[allow(improper_ctypes)]
pub(crate) fn LLVMRustCoverageWriteFunctionMappingsToBuffer(
VirtualFileMappingIDs: *const c_uint,
NumVirtualFileMappingIDs: size_t,
Expand All @@ -1824,13 +1820,10 @@ unsafe extern "C" {
) -> &Value;
pub(crate) fn LLVMRustCoverageHashBytes(Bytes: *const c_char, NumBytes: size_t) -> u64;

#[allow(improper_ctypes)]
pub(crate) fn LLVMRustCoverageWriteCovmapSectionNameToString(M: &Module, OutStr: &RustString);

#[allow(improper_ctypes)]
pub(crate) fn LLVMRustCoverageWriteCovfunSectionNameToString(M: &Module, OutStr: &RustString);

#[allow(improper_ctypes)]
pub(crate) fn LLVMRustCoverageWriteCovmapVarNameToString(OutStr: &RustString);

pub(crate) fn LLVMRustCoverageMappingVersion() -> u32;
Expand Down Expand Up @@ -2185,14 +2178,11 @@ unsafe extern "C" {
pub fn LLVMRustDIBuilderCreateOpPlusUconst() -> u64;
pub fn LLVMRustDIBuilderCreateOpLLVMFragment() -> u64;

#[allow(improper_ctypes)]
pub fn LLVMRustWriteTypeToString(Type: &Type, s: &RustString);
#[allow(improper_ctypes)]
pub fn LLVMRustWriteValueToString(value_ref: &Value, s: &RustString);

pub fn LLVMRustHasFeature(T: &TargetMachine, s: *const c_char) -> bool;

#[allow(improper_ctypes)]
pub(crate) fn LLVMRustPrintTargetCPUs(TM: &TargetMachine, OutStr: &RustString);
pub fn LLVMRustGetTargetFeaturesCount(T: &TargetMachine) -> size_t;
pub fn LLVMRustGetTargetFeature(
Expand Down Expand Up @@ -2297,10 +2287,8 @@ unsafe extern "C" {
pub fn LLVMRustArchiveIteratorFree<'a>(AIR: &'a mut ArchiveIterator<'a>);
pub fn LLVMRustDestroyArchive(AR: &'static mut Archive);

#[allow(improper_ctypes)]
pub fn LLVMRustWriteTwineToString(T: &Twine, s: &RustString);

#[allow(improper_ctypes)]
pub fn LLVMRustUnpackOptimizationDiagnostic<'a>(
DI: &'a DiagnosticInfo,
pass_name_out: &RustString,
Expand All @@ -2318,7 +2306,6 @@ unsafe extern "C" {
message_out: &mut Option<&'a Twine>,
);

#[allow(improper_ctypes)]
pub fn LLVMRustWriteDiagnosticInfoToString(DI: &DiagnosticInfo, s: &RustString);
pub fn LLVMRustGetDiagInfoKind(DI: &DiagnosticInfo) -> DiagnosticKind;

Expand All @@ -2327,7 +2314,6 @@ unsafe extern "C" {
cookie_out: &mut c_uint,
) -> &'a SMDiagnostic;

#[allow(improper_ctypes)]
pub fn LLVMRustUnpackSMDiagnostic(
d: &SMDiagnostic,
message_out: &RustString,
Expand Down Expand Up @@ -2374,7 +2360,6 @@ unsafe extern "C" {
pub fn LLVMRustModuleBufferLen(p: &ModuleBuffer) -> usize;
pub fn LLVMRustModuleBufferFree(p: &'static mut ModuleBuffer);
pub fn LLVMRustModuleCost(M: &Module) -> u64;
#[allow(improper_ctypes)]
pub fn LLVMRustModuleInstructionStats(M: &Module, Str: &RustString);

pub fn LLVMRustThinLTOBufferCreate(
Expand Down Expand Up @@ -2427,7 +2412,6 @@ unsafe extern "C" {
bytecode_len: usize,
) -> bool;
pub fn LLVMRustLinkerFree<'a>(linker: &'a mut Linker<'a>);
#[allow(improper_ctypes)]
pub fn LLVMRustComputeLTOCacheKey(
key_out: &RustString,
mod_id: *const c_char,
Expand All @@ -2450,7 +2434,6 @@ unsafe extern "C" {
pgo_available: bool,
);

#[allow(improper_ctypes)]
pub fn LLVMRustGetMangledName(V: &Value, out: &RustString);

pub fn LLVMRustGetElementTypeArgIndex(CallSite: &Value) -> i32;
Expand Down
9 changes: 2 additions & 7 deletions compiler/rustc_codegen_llvm/src/llvm/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#![allow(non_snake_case)]

use std::cell::RefCell;
use std::ffi::{CStr, CString};
use std::ops::Deref;
use std::ptr;
Expand Down Expand Up @@ -301,15 +300,11 @@ pub fn set_value_name(value: &Value, name: &[u8]) {
}

pub fn build_string(f: impl FnOnce(&RustString)) -> Result<String, FromUtf8Error> {
let sr = RustString { bytes: RefCell::new(Vec::new()) };
f(&sr);
String::from_utf8(sr.bytes.into_inner())
String::from_utf8(RustString::build_byte_buffer(f))
}

pub fn build_byte_buffer(f: impl FnOnce(&RustString)) -> Vec<u8> {
let sr = RustString { bytes: RefCell::new(Vec::new()) };
f(&sr);
sr.bytes.into_inner()
RustString::build_byte_buffer(f)
}

pub fn twine_to_string(tr: &Twine) -> String {
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_llvm/llvm-wrapper/LLVMWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,9 @@ typedef struct OpaqueRustString *RustStringRef;
typedef struct LLVMOpaqueTwine *LLVMTwineRef;
typedef struct LLVMOpaqueSMDiagnostic *LLVMSMDiagnosticRef;

extern "C" void LLVMRustStringWriteImpl(RustStringRef Str, const char *Ptr,
size_t Size);
extern "C" void LLVMRustStringWriteImpl(RustStringRef buf,
const char *slice_ptr,
size_t slice_len);

class RawRustStringOstream : public llvm::raw_ostream {
RustStringRef Str;
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1624,5 +1624,6 @@ extern "C" void LLVMRustComputeLTOCacheKey(RustStringRef KeyOut,
CfiFunctionDefs, CfiFunctionDecls);
#endif

LLVMRustStringWriteImpl(KeyOut, Key.c_str(), Key.size());
auto OS = RawRustStringOstream(KeyOut);
OS << Key.str();
}
4 changes: 2 additions & 2 deletions compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1510,8 +1510,8 @@ LLVMRustUnpackSMDiagnostic(LLVMSMDiagnosticRef DRef, RustStringRef MessageOut,
const SourceMgr &LSM = *D.getSourceMgr();
const MemoryBuffer *LBuf =
LSM.getMemoryBuffer(LSM.FindBufferContainingLoc(D.getLoc()));
LLVMRustStringWriteImpl(BufferOut, LBuf->getBufferStart(),
LBuf->getBufferSize());
auto BufferOS = RawRustStringOstream(BufferOut);
BufferOS << LBuf->getBuffer();

*LocOut = D.getLoc().getPointer() - LBuf->getBufferStart();

Expand Down
69 changes: 51 additions & 18 deletions compiler/rustc_llvm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,42 +2,75 @@
#![allow(internal_features)]
#![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
#![doc(rust_logo)]
#![feature(extern_types)]
#![feature(rustdoc_internals)]
#![warn(unreachable_pub)]
// tidy-alphabetical-end

// NOTE: This crate only exists to allow linking on mingw targets.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what this was originally referring to, but it's pretty clearly no longer true of the present crate.


use std::cell::RefCell;
use std::slice;
use std::{ptr, slice};

use libc::{c_char, size_t};
use libc::size_t;

#[repr(C)]
pub struct RustString {
pub bytes: RefCell<Vec<u8>>,
unsafe extern "C" {
/// Opaque type that allows C++ code to write bytes to a Rust-side buffer,
/// in conjunction with `RawRustStringOstream`. Use this as `&RustString`
/// (Rust) and `RustStringRef` (C++) in FFI signatures.
pub type RustString;
}

impl RustString {
pub fn len(&self) -> usize {
self.bytes.borrow().len()
pub fn build_byte_buffer(closure: impl FnOnce(&Self)) -> Vec<u8> {
let buf = RustStringInner::default();
closure(buf.as_opaque());
buf.into_inner()
}
}

pub fn is_empty(&self) -> bool {
self.bytes.borrow().is_empty()
/// Underlying implementation of [`RustString`].
///
/// Having two separate types makes it possible to use the opaque [`RustString`]
/// in FFI signatures without `improper_ctypes` warnings. This is a workaround
/// for the fact that there is no way to opt out of `improper_ctypes` when
/// _declaring_ a type (as opposed to using that type).
#[derive(Default)]
struct RustStringInner {
bytes: RefCell<Vec<u8>>,
}

impl RustStringInner {
fn as_opaque(&self) -> &RustString {
let ptr: *const RustStringInner = ptr::from_ref(self);
// We can't use `ptr::cast` here because extern types are `!Sized`.
let ptr = ptr as *const RustString;
unsafe { &*ptr }
}

fn from_opaque(opaque: &RustString) -> &Self {
// SAFETY: A valid `&RustString` must have been created via `as_opaque`.
let ptr: *const RustString = ptr::from_ref(opaque);
let ptr: *const RustStringInner = ptr.cast();
unsafe { &*ptr }
}

fn into_inner(self) -> Vec<u8> {
self.bytes.into_inner()
}
}

/// Appending to a Rust string -- used by RawRustStringOstream.
/// Appends the contents of a byte slice to a [`RustString`].
///
/// This function is implemented in `rustc_llvm` so that the C++ code in this
/// crate can link to it directly, without an implied link-time dependency on
/// `rustc_codegen_llvm`.
#[unsafe(no_mangle)]
pub unsafe extern "C" fn LLVMRustStringWriteImpl(
sr: &RustString,
ptr: *const c_char,
size: size_t,
buf: &RustString,
slice_ptr: *const u8, // Same ABI as `*const c_char`
slice_len: size_t,
) {
let slice = unsafe { slice::from_raw_parts(ptr as *const u8, size) };

sr.bytes.borrow_mut().extend_from_slice(slice);
let slice = unsafe { slice::from_raw_parts(slice_ptr, slice_len) };
RustStringInner::from_opaque(buf).bytes.borrow_mut().extend_from_slice(slice);
}

/// Initialize targets enabled by the build script via `cfg(llvm_component = "...")`.
Expand Down
Loading