Skip to content

Commit c458496

Browse files
authored
transpile: expand --translate-const-macros conservative to a lot more CExprKinds (#1306)
* Fixes #803. * Supersedes #810. We do this by recursively checking whether an expression is `const`. This is generally done in a conservative manner, modulo the `ExplicitCast` bug (#853), non-`const` `sizeof(VLA)`s, and `f128`'s non-`const` methods (#1262). For #853, this does generate incorrect code even on `conservative`, but I'll work on fixing that next. Also, because we now allow certain operations that are `unsafe`, like ptr arithmetic, we wrap all `const`s in an `unsafe` block. This is similar to how all `fn`s we generate are marked `unsafe fn`s even if they don't contain any `unsafe` operations within them. We can improve this, but for now, this works and is correct. Also, the output was being non-deterministic due to the usage of `HashMap`s for macro maps like `macro_invocations`, so I changed these to `IndexMap`s that are order-preserving.
2 parents 1ef5529 + ecddc95 commit c458496

File tree

7 files changed

+347
-190
lines changed

7 files changed

+347
-190
lines changed

c2rust-transpile/src/c_ast/mod.rs

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,78 @@ impl TypedAstContext {
624624
}
625625
}
626626

627+
/// Pessimistically try to check if an expression is `const`.
628+
/// If it's not, or we can't tell if it is, return `false`.
629+
///
630+
/// This should be a top-down, pessimistic/conservative analysis.
631+
pub fn is_const_expr(&self, expr: CExprId) -> bool {
632+
let is_const = |expr| self.is_const_expr(expr);
633+
634+
use CExprKind::*;
635+
match self[expr].kind {
636+
// A literal is always `const`.
637+
Literal(_, _) => true,
638+
// Unary ops should be `const`.
639+
// TODO handle `f128` or use the primitive type.
640+
Unary(_, _, expr, _) => is_const(expr),
641+
UnaryType(_, _, _, _) => false, // TODO disabled for now as tests are broken
642+
// Not sure what a `None` `CExprId` means here
643+
// or how to detect a `sizeof` of a VLA, which is non-`const`,
644+
// although it seems we don't handle `sizeof(VLAs)`
645+
// correctly in macros elsewhere already.
646+
#[allow(unreachable_patterns)]
647+
UnaryType(_, _, expr, _) => expr.map_or(true, is_const),
648+
// Not sure what a `OffsetOfKind::Variable` means.
649+
OffsetOf(_, _) => true,
650+
// `ptr::offset` (ptr `BinOp::Add`) was `const` stabilized in `1.61.0`.
651+
// `ptr::offset_from` (ptr `BinOp::Subtract`) was `const` stabilized in `1.65.0`.
652+
// TODO `f128` is not yet handled, as we should eventually
653+
// switch to the (currently unstable) `f128` primitive type (#1262).
654+
Binary(_, _, lhs, rhs, _, _) => is_const(lhs) && is_const(rhs),
655+
ImplicitCast(_, _, CastKind::ArrayToPointerDecay, _, _) => false, // TODO disabled for now as tests are broken
656+
// `as` casts are always `const`.
657+
ImplicitCast(_, expr, _, _, _) => is_const(expr),
658+
// `as` casts are always `const`.
659+
// TODO This is `const`, although there's a bug #853.
660+
ExplicitCast(_, expr, _, _, _) => is_const(expr),
661+
// This is used in `const` locations like `match` patterns and array lengths, so it must be `const`.
662+
ConstantExpr(_, _, _) => true,
663+
// A reference in an already otherwise `const` context should be `const` itself.
664+
DeclRef(_, _, _) => true,
665+
Call(_, fn_expr, ref args) => {
666+
let is_const_fn = false; // TODO detect which `fn`s are `const`.
667+
is_const(fn_expr) && args.iter().copied().all(is_const) && is_const_fn
668+
}
669+
Member(_, expr, _, _, _) => is_const(expr),
670+
ArraySubscript(_, array, index, _) => is_const(array) && is_const(index),
671+
Conditional(_, cond, if_true, if_false) => {
672+
is_const(cond) && is_const(if_true) && is_const(if_false)
673+
}
674+
BinaryConditional(_, cond, if_false) => is_const(cond) && is_const(if_false),
675+
InitList(_, ref inits, _, _) => inits.iter().copied().all(is_const),
676+
ImplicitValueInit(_) => true,
677+
Paren(_, expr) => is_const(expr),
678+
CompoundLiteral(_, expr) => is_const(expr),
679+
Predefined(_, expr) => is_const(expr),
680+
Statements(_, stmt) => self.is_const_stmt(stmt),
681+
VAArg(_, expr) => is_const(expr),
682+
// SIMD is not yet `const` in Rust.
683+
ShuffleVector(_, _) | ConvertVector(_, _) => false,
684+
DesignatedInitExpr(_, _, expr) => is_const(expr),
685+
Choose(_, cond, if_true, if_false, _) => {
686+
is_const(cond) && is_const(if_true) && is_const(if_false)
687+
}
688+
// Atomics are not yet `const` in Rust.
689+
Atomic { .. } => false,
690+
BadExpr => false,
691+
}
692+
}
693+
694+
pub fn is_const_stmt(&self, _stmt: CStmtId) -> bool {
695+
// TODO
696+
false
697+
}
698+
627699
pub fn prune_unwanted_decls(&mut self, want_unused_functions: bool) {
628700
// Starting from a set of root declarations, walk each one to find declarations it
629701
// depends on. Then walk each of those, recursively.

c2rust-transpile/src/translator/mod.rs

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2123,15 +2123,28 @@ impl<'c> Translation<'c> {
21232123

21242124
/// Determine if we're able to convert this const macro expansion.
21252125
fn can_convert_const_macro_expansion(&self, expr_id: CExprId) -> TranslationResult<()> {
2126-
let kind = &self.ast_context[expr_id].kind;
21272126
match self.tcfg.translate_const_macros {
21282127
TranslateMacros::None => Err(format_err!("translate_const_macros is None"))?,
2129-
TranslateMacros::Conservative => match *kind {
2130-
CExprKind::Literal(..) => Ok(()), // Literals are leaf expressions, so they should always be const-compatible.
2131-
_ => Err(format_err!(
2132-
"conservative const macros don't yet allow {kind:?}"
2133-
))?,
2134-
},
2128+
TranslateMacros::Conservative => {
2129+
// TODO We still allow `CExprKind::ExplicitCast`s
2130+
// even though they're broken (see #853).
2131+
2132+
// This is a top-down, pessimistic/conservative analysis.
2133+
// This is somewhat duplicative of `fn convert_expr` simply checking
2134+
// `ExprContext::is_const` and returning errors where the expr is not `const`,
2135+
// which is an non-conservative analysis scattered across all of the `fn convert_*`s.
2136+
// That's what's done for `TranslateMacros::Experimental`,
2137+
// as opposed to the conservative analysis done here for `TranslateMacros::Conservative`.
2138+
// When the conservative analysis is incomplete,
2139+
// it won't translate the macro, but the result will compile.
2140+
// But when the non-conservative analysis is incomplete,
2141+
// the resulting code may not transpile,
2142+
// which is why the conservative analysis is used for `TranslateMacros::Conservative`.
2143+
if !self.ast_context.is_const_expr(expr_id) {
2144+
Err(format_err!("non-const expr {expr_id:?}"))?;
2145+
}
2146+
Ok(())
2147+
}
21352148
TranslateMacros::Experimental => Ok(()),
21362149
}
21372150
}

c2rust-transpile/tests/snapshots/macros.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,9 @@ struct fn_ptrs {
289289

290290
typedef int (*fn_ptr_ty)(char);
291291

292-
const struct fn_ptrs fns = {NULL, NULL, NULL};
292+
// TODO Skip for now since it uses `libc`, which we don't test in snapshots.
293+
// const struct fn_ptrs fns = {NULL, NULL, NULL};
294+
const struct fn_ptrs fns = {};
293295

294296
// Make sure we can't refer to globals in a const macro
295297
#define GLOBAL_REF &fns
@@ -366,6 +368,4 @@ int local_fn(void) { return 1234; }
366368

367369
int use_local_value(void) { return LOCAL_VALUE; }
368370

369-
bool use_portable_type(uintptr_t len) {
370-
return len <= UINTPTR_MAX / 2;
371-
}
371+
bool use_portable_type(uintptr_t len) { return len <= UINTPTR_MAX / 2; }
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
#include <errno.h>
2+
#include <stdbool.h>
3+
#include <string.h>
4+
5+
bool errno_is_error() { return errno != 0; }
6+
7+
int size_of_const() {
8+
int a[10];
9+
#define SIZE sizeof(a)
10+
return SIZE;
11+
}
12+
13+
#if 0
14+
// TODO VLA error
15+
int size_of_dynamic(int n) {
16+
int a[n];
17+
#define SIZE sizeof(a)
18+
return SIZE;
19+
}
20+
#endif
21+
22+
// From Lua's `lobject.c`.
23+
24+
#define POS "\"]"
25+
/* number of chars of a literal string without the ending \0 */
26+
#define LL(x) (sizeof(x) / sizeof(char) - 1)
27+
28+
void memcpy_str_literal(char *out) {
29+
memcpy(out, POS, (LL(POS) + 1) * sizeof(char));
30+
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
---
2+
source: c2rust-transpile/tests/snapshots.rs
3+
expression: cat tests/snapshots/os-specific/macros.linux.rs
4+
input_file: c2rust-transpile/tests/snapshots/os-specific/macros.c
5+
---
6+
#![allow(
7+
dead_code,
8+
mutable_transmutes,
9+
non_camel_case_types,
10+
non_snake_case,
11+
non_upper_case_globals,
12+
unused_assignments,
13+
unused_mut
14+
)]
15+
extern "C" {
16+
fn __errno_location() -> *mut std::ffi::c_int;
17+
fn memcpy(
18+
__dest: *mut std::ffi::c_void,
19+
__src: *const std::ffi::c_void,
20+
__n: size_t,
21+
) -> *mut std::ffi::c_void;
22+
}
23+
pub type size_t = usize;
24+
#[no_mangle]
25+
pub unsafe extern "C" fn errno_is_error() -> bool {
26+
return *__errno_location() != 0 as std::ffi::c_int;
27+
}
28+
#[no_mangle]
29+
pub unsafe extern "C" fn size_of_const() -> std::ffi::c_int {
30+
let mut a: [std::ffi::c_int; 10] = [0; 10];
31+
return ::core::mem::size_of::<[std::ffi::c_int; 10]>() as std::ffi::c_int;
32+
}
33+
#[no_mangle]
34+
pub unsafe extern "C" fn memcpy_str_literal(mut out: *mut std::ffi::c_char) {
35+
memcpy(
36+
out as *mut std::ffi::c_void,
37+
b"\"]\0" as *const u8 as *const std::ffi::c_char as *const std::ffi::c_void,
38+
(::core::mem::size_of::<[std::ffi::c_char; 3]>() as size_t)
39+
.wrapping_div(::core::mem::size_of::<std::ffi::c_char>() as size_t)
40+
.wrapping_sub(1 as size_t)
41+
.wrapping_add(1 as size_t)
42+
.wrapping_mul(::core::mem::size_of::<std::ffi::c_char>() as size_t),
43+
);
44+
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
source: c2rust-transpile/tests/snapshots.rs
3+
expression: cat tests/snapshots/os-specific/macros.macos.rs
4+
input_file: c2rust-transpile/tests/snapshots/os-specific/macros.c
5+
---
6+
#![allow(
7+
dead_code,
8+
mutable_transmutes,
9+
non_camel_case_types,
10+
non_snake_case,
11+
non_upper_case_globals,
12+
unused_assignments,
13+
unused_mut
14+
)]
15+
extern "C" {
16+
fn __error() -> *mut std::ffi::c_int;
17+
fn memcpy(
18+
__dst: *mut std::ffi::c_void,
19+
__src: *const std::ffi::c_void,
20+
__n: size_t,
21+
) -> *mut std::ffi::c_void;
22+
}
23+
pub type __darwin_size_t = usize;
24+
pub type size_t = __darwin_size_t;
25+
#[no_mangle]
26+
pub unsafe extern "C" fn errno_is_error() -> bool {
27+
return *__error() != 0 as std::ffi::c_int;
28+
}
29+
#[no_mangle]
30+
pub unsafe extern "C" fn size_of_const() -> std::ffi::c_int {
31+
let mut a: [std::ffi::c_int; 10] = [0; 10];
32+
return ::core::mem::size_of::<[std::ffi::c_int; 10]>() as std::ffi::c_int;
33+
}
34+
#[no_mangle]
35+
pub unsafe extern "C" fn memcpy_str_literal(mut out: *mut std::ffi::c_char) {
36+
memcpy(
37+
out as *mut std::ffi::c_void,
38+
b"\"]\0" as *const u8 as *const std::ffi::c_char as *const std::ffi::c_void,
39+
(::core::mem::size_of::<[std::ffi::c_char; 3]>() as size_t)
40+
.wrapping_div(::core::mem::size_of::<std::ffi::c_char>() as size_t)
41+
.wrapping_sub(1 as size_t)
42+
.wrapping_add(1 as size_t)
43+
.wrapping_mul(::core::mem::size_of::<std::ffi::c_char>() as size_t),
44+
);
45+
}

0 commit comments

Comments
 (0)