Skip to content

transpile: Refactor and improve address-of handling #1982

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.

Sign up for GitHub

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

Draft
Rua wants to merge 12 commits into
base: master
Choose a base branch
from
Draft

transpile: Refactor and improve address-of handling #1982

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b889a6a
transpile: Fix indentation in `macros.c` snapshot test
Rua Sep 3, 2026
c521ec5
transpile: Add tests to `macro.c` for mutability of compound literals
Rua Sep 3, 2026
821e105
transpile: In `macros.c`, put statics first to avoid #1994
Rua Sep 3, 2026
848ff3e
transpile: Conservatively disallow mutable lvalues inside macros
Rua Sep 3, 2026
e78645d
transpile: Assert that address-of string literal or macro is immutable
Rua Sep 3, 2026
f0741ea
transpile: Use `ptr::from_ref` to cast references to pointers
Rua Aug 24, 2026
2fb258d
transpile: Remove `decay_ref`, which is now written but never read
Rua Aug 25, 2026
df17903
transpile: Refactor `convert_address_of_common`
Rua Aug 25, 2026
16f1e36
transpile: Only borrow const and cast in 2021 edition
Rua Sep 2, 2026
dd62f81
transpile: In `convert_address_of_common`, cast based on type kinds
Rua Aug 25, 2026
c85778d
transpile: Merge `convert_array_to_pointer_decay` and `convert_addres…
Rua Aug 25, 2026
e7525b5
transpile: Move `is_function_pointer` check to `make_address_of`
Rua Aug 25, 2026
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
2 changes: 1 addition & 1 deletion c2rust-ast-builder/src/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub mod properties {
fn to_token(&self) -> Option<Self::Token>;
}

#[derive(Debug, Copy, Clone)]
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum Mutability {
Mutable,
Immutable,
Expand Down
12 changes: 10 additions & 2 deletions c2rust-transpile/src/c_ast/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3360,9 +3360,17 @@ impl CTypeKind {
}

/// Return the element type of a pointer or array
pub fn element_ty(&self) -> Option<CTypeId> {
pub(crate) fn array_element_type(&self) -> Option<CTypeId> {
Some(match *self {
Self::Pointer(ty) => ty.ctype,
Self::ConstantArray(ty, _) => ty,
Self::IncompleteArray(ty) => ty,
Self::VariableArray(ty, _) => ty,
_ => return None,
})
}

pub(crate) fn array_element_type_mut(&mut self) -> Option<&mut CTypeId> {
Some(match self {
Self::ConstantArray(ty, _) => ty,
Self::IncompleteArray(ty) => ty,
Self::VariableArray(ty, _) => ty,
Expand Down
5 changes: 1 addition & 4 deletions c2rust-transpile/src/translator/functions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ impl<'c> Translation<'c> {

pub fn convert_function_call(
&self,
mut ctx: ExprContext,
ctx: ExprContext,
func: CExprId,
args: &[CExprId],
call_expr_ty: CQualTypeId,
Expand Down Expand Up @@ -447,9 +447,6 @@ impl<'c> Translation<'c> {
};

let call = func.and_then_try(|func| {
// We want to decay refs only when function is variadic
ctx.decay_ref = DecayRef::from(is_variadic);

let args = self.convert_call_args(ctx.used(), args, arg_tys.as_deref(), is_variadic)?;

let call_expr = args.map(|args| mk().call_expr(func, args));
Expand Down
6 changes: 5 additions & 1 deletion c2rust-transpile/src/translator/literals.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,13 @@ impl<'c> Translation<'c> {
val: CExprId,
override_ty: Option<CQualTypeId>,
) -> TranslationResult<WithStmts<Box<Expr>>> {
if !qty.qualifiers.is_const && ctx.expanding_macro.is_some() {
return Err("mutable lvalues are not supported inside macros".into());
}

// C compound literals are lvalues, but equivalent Rust expressions generally are not.
// So if an address is needed, store it in an intermediate variable first.
if !ctx.needs_address || ctx.expanding_macro.is_some() {
if !ctx.needs_address {
return self.convert_expr(ctx, val, override_ty);
}

Expand Down
81 changes: 15 additions & 66 deletions c2rust-transpile/src/translator/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,45 +74,6 @@ struct Import {
ident_name: String,
}

#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum DecayRef {
Yes,
Default,
No,
}

impl DecayRef {
// Here we give intrinsic meaning to default to equate to yes/true
// when actually evaluated
pub fn is_yes(&self) -> bool {
match self {
DecayRef::Yes => true,
DecayRef::Default => true,
DecayRef::No => false,
}
}

#[inline]
pub fn is_no(&self) -> bool {
!self.is_yes()
}

pub fn set_default_to_no(&mut self) {
if *self == DecayRef::Default {
*self = DecayRef::No;
}
}
}

impl From<bool> for DecayRef {
fn from(b: bool) -> Self {
match b {
true => DecayRef::Yes,
false => DecayRef::No,
}
}
}

#[derive(Debug, Copy, Clone)]
pub enum ReplaceMode {
None,
Expand All @@ -138,7 +99,6 @@ pub struct ExprContext {
#[allow(dead_code)]
is_static: bool,

decay_ref: DecayRef,
is_bitfield_write: bool,

/// We will be referring to the expression by address. In this context we
Expand Down Expand Up @@ -166,12 +126,7 @@ impl ExprContext {
pub fn is_unused(&self) -> bool {
!self.used
}
pub fn decay_ref(self) -> Self {
ExprContext {
decay_ref: DecayRef::Yes,
..self
}
}

pub fn const_(self) -> Self {
ExprContext {
is_const: true,
Expand Down Expand Up @@ -880,7 +835,6 @@ pub fn translate(
is_const: false,
is_pattern: false,
is_static: false,
decay_ref: DecayRef::Default,
is_bitfield_write: false,
needs_address: false,
expanding_macro: None,
Expand Down Expand Up @@ -1735,7 +1689,7 @@ impl<'c> Translation<'c> {
pub fn use_feature(&self, feature: &'static str) {
if matches!(
feature,
"asm" | "inline_const" | "label_break_value" | "raw_ref_op"
"asm" | "inline_const" | "label_break_value" | "ptr_from_ref" | "raw_ref_op"
) && self.tcfg.edition >= Edition2024
{
return;
Expand Down Expand Up @@ -2479,7 +2433,7 @@ impl<'c> Translation<'c> {

let null_pointer_case =
|ptr: CExprId, is_null: bool| -> TranslationResult<WithStmts<Box<Expr>>> {
let val = self.convert_expr(ctx.used().decay_ref(), ptr, None)?;
let val = self.convert_expr(ctx.used(), ptr, None)?;
let ptr_type = self
.ast_context
.index_unwrap_parens(ptr)
Expand Down Expand Up @@ -2530,11 +2484,7 @@ impl<'c> Translation<'c> {
}

_ => {
// DecayRef could (and probably should) be Default instead of Yes here; however, as noted
// in https://github.com/rust-lang/rust/issues/53772, you cant compare a reference (lhs) to
// a ptr (rhs) (even though the reverse works!). We could also be smarter here and just
// specify Yes for that particular case, given enough analysis.
let val = self.convert_expr(ctx.used().decay_ref(), cond_id, None)?;
let val = self.convert_expr(ctx.used(), cond_id, None)?;
val.try_map(|e| self.match_bool(ctx, target, ty_id, e))
}
}
Expand Down Expand Up @@ -2976,7 +2926,6 @@ impl<'c> Translation<'c> {
};
}

// ref decayed ptrs generally need a type annotation
if let Some(CExprKind::Unary(_, CUnOp::AddressOf, _, _)) = initializer_kind
{
return true;
Expand Down Expand Up @@ -3544,8 +3493,8 @@ impl<'c> Translation<'c> {
matches!(expr_kind, CExprKind::ExplicitCast(..)),
),

Unary(result_type_id, op, arg, _lrvalue) => {
self.convert_unary_operator(ctx, override_ty, result_type_id, op, arg)
Unary(result_type_id, op, arg, lrvalue) => {
self.convert_unary_operator(ctx, override_ty, result_type_id, op, arg, lrvalue)
}

Conditional(ty, cond, lhs, rhs) => {
Expand Down Expand Up @@ -3637,8 +3586,8 @@ impl<'c> Translation<'c> {
)
.map_err(|e| e.add_loc(self.ast_context.display_loc(src_loc))),

ArraySubscript(_, lhs, rhs, lrvalue) => self
.convert_array_subscript(ctx, override_ty, lhs, rhs, lrvalue, true)
ArraySubscript(result_type_id, lhs, rhs, lrvalue) => self
.convert_array_subscript(ctx, override_ty, result_type_id, lhs, rhs, lrvalue, true)
.map_err(|e| e.add_loc(self.ast_context.display_loc(src_loc))),

Call(call_expr_ty, func, ref args) => {
Expand Down Expand Up @@ -3759,6 +3708,8 @@ impl<'c> Translation<'c> {
.get_decl(&decl_id)
.ok_or_else(|| format_err!("Missing declref {:?}", decl_id))?
.kind;

#[allow(unreachable_code)] // TODO temporary (see below).
if ctx.expanding_macro.is_some() {
// TODO Determining which declarations have been declared within the scope of the const macro expr
// vs. which are out-of-scope of the const macro is non-trivial,
Expand All @@ -3768,7 +3719,10 @@ impl<'c> Translation<'c> {
"Cannot yet refer to declarations in a const expr",
));

#[allow(unreachable_code)] // TODO temporary (see above).
if matches!(lrvalue, LRValue::LValue) && !result_type_id.qualifiers.is_const {
return Err("mutable lvalues are not supported inside macros".into());
}

if let CDeclKind::Variable {
has_static_duration: true,
..
Expand Down Expand Up @@ -4100,11 +4054,6 @@ impl<'c> Translation<'c> {
}

match kind {
// A reference must be decayed if a bitcast is required. Const casts in
// LLVM 8 are now NoOp casts, so we need to include it as well.
CastKind::BitCast | CastKind::PointerToIntegral | CastKind::NoOp => {
ctx.decay_ref = DecayRef::Yes
}
CastKind::ArrayToPointerDecay
| CastKind::FunctionToPointerDecay
| CastKind::BuiltinFnToFnPtr => {
Expand Down Expand Up @@ -4356,7 +4305,7 @@ impl<'c> Translation<'c> {
}

CastKind::ArrayToPointerDecay => {
self.convert_array_to_pointer_decay(ctx, source_cty, target_cty, val, expr)
self.make_address_of(ctx, target_cty, source_cty, expr, val, true)
}

CastKind::NullToPointer => {
Expand Down
30 changes: 5 additions & 25 deletions c2rust-transpile/src/translator/operators.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use super::*;
impl<'c> Translation<'c> {
pub fn convert_binary_expr(
&self,
mut ctx: ExprContext,
ctx: ExprContext,
expected_type_id: Option<CQualTypeId>,
result_type_id: CQualTypeId,
op: CBinOp,
Expand Down Expand Up @@ -53,14 +53,6 @@ impl<'c> Translation<'c> {
),

_ => {
// Comparing references to pointers isn't consistently supported by rust
// and so we need to decay references to pointers to do so. See
// https://github.com/rust-lang/rust/issues/53772. This might be removable
// once the above issue is resolved.
if op == CBinOp::EqualEqual || op == CBinOp::NotEqual {
ctx = ctx.decay_ref();
}

let lhs_kind = &self.ast_context.index_unwrap_parens(lhs).kind;
let mut lhs_type_id = lhs_kind.get_qual_type().ok_or_else(|| {
format_translation_err!(
Expand Down Expand Up @@ -119,26 +111,13 @@ impl<'c> Translation<'c> {
.and_then_try(|_| self.convert_expr(ctx, rhs, Some(rhs_type_id)))?
.map(|_| self.panic_or_err("Binary expression is not supposed to be used")))
} else {
let rhs_ctx = ctx;

// When we use methods on pointers (ie wrapping_offset_from or offset)
// we must ensure we have an explicit raw ptr for the self param, as
// self references do not decay
if op.is_pointer_arithmetic() {
let ty_kind = &self.ast_context.resolve_type(lhs_type_id.ctype).kind;

if let CTypeKind::Pointer(_) = ty_kind {
ctx = ctx.decay_ref();
}
}

// Using `.is_none()` and `.is_some()` for null comparison means we don't
// have to rely on `trait PartialEq` as much and it is also more idiomatic.
if matches!(op, CBinOp::EqualEqual | CBinOp::NotEqual) {
let is_null = op == CBinOp::EqualEqual;

if self.ast_context.is_null_expr(lhs) {
let val = self.convert_expr(rhs_ctx, rhs, Some(rhs_type_id))?;
let val = self.convert_expr(ctx, rhs, Some(rhs_type_id))?;
let val = val.try_map(|rhs_rs| {
self.convert_pointer_is_null(
ctx,
Expand All @@ -163,7 +142,7 @@ impl<'c> Translation<'c> {
}

let lhs_val = self.convert_expr(ctx, lhs, Some(lhs_type_id))?;
let rhs_val = self.convert_expr(rhs_ctx, rhs, Some(rhs_type_id))?;
let rhs_val = self.convert_expr(ctx, rhs, Some(rhs_type_id))?;

lhs_val.zip(rhs_val).and_then_try(|(lhs_val, rhs_val)| {
self.convert_binary_operator(
Expand Down Expand Up @@ -570,6 +549,7 @@ impl<'c> Translation<'c> {
result_type_id: CQualTypeId,
op: CUnOp,
arg: CExprId,
lrvalue: LRValue,
) -> TranslationResult<WithStmts<Box<Expr>>> {
let expr_type_id = expected_type_id.unwrap_or(result_type_id);
let mut unary = match op {
Expand All @@ -582,7 +562,7 @@ impl<'c> Translation<'c> {
self.convert_indecrement_operator(ctx, expected_type_id, result_type_id, op, arg)
}

CUnOp::Deref => self.convert_deref(ctx, expr_type_id, arg),
CUnOp::Deref => self.convert_deref(ctx, expr_type_id, arg, lrvalue),
CUnOp::Plus => self.convert_expr(ctx.used(), arg, expected_type_id), // promotion is explicit in the clang AST

CUnOp::Negate => self.convert_negate_operator(ctx, expr_type_id, arg),
Expand Down
Loading
Loading