Skip to content

Commit 46a9b91

Browse files
committed
transpile: expand --translate-const-macros conservative to a lot more CExprKinds
We do this by recursively checking whether an expression is `const`. This is generally done in a conservative manner, modulo a few bugs and unhandled things, namely: * the `ExplicitCast` bug (#853) * non-`const` `sizeof(VLA)`s * `sizeof`s and other `UnaryType` exprs (e.x. `alignof`) * `f128`'s non-`const` methods (#1262) * statements
1 parent 195e8cd commit 46a9b91

File tree

3 files changed

+210
-186
lines changed

3 files changed

+210
-186
lines changed

c2rust-transpile/src/c_ast/mod.rs

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

c2rust-transpile/src/translator/mod.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2123,15 +2123,16 @@ 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+
if !self.ast_context.is_const_expr(expr_id) {
2132+
Err(format_err!("non-const expr {expr_id:?}"))?;
2133+
}
2134+
Ok(())
2135+
}
21352136
TranslateMacros::Experimental => Ok(()),
21362137
}
21372138
}

0 commit comments

Comments
 (0)