Skip to content

Commit f3966c2

Browse files
RenjiSannRenjiSann
committed
coverage: Prepare MIR boolean expression building for condition coverage
1 parent ae60990 commit f3966c2

File tree

1 file changed

+82
-35
lines changed
  • compiler/rustc_mir_build/src/build/expr

1 file changed

+82
-35
lines changed

‎compiler/rustc_mir_build/src/build/expr/into.rs

+82-35
Original file line numberDiff line numberDiff line change
@@ -148,43 +148,90 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
148148
ExprKind::LogicalOp { op, lhs, rhs } => {
149149
let condition_scope = this.local_scope();
150150
let source_info = this.source_info(expr.span);
151-
// We first evaluate the left-hand side of the predicate ...
152-
let (then_block, else_block) =
153-
this.in_if_then_scope(condition_scope, expr.span, |this| {
154-
this.then_else_break(
155-
block,
156-
lhs,
157-
Some(condition_scope), // Temp scope
158-
source_info,
159-
// This flag controls how inner `let` expressions are lowered,
160-
// but either way there shouldn't be any of those in here.
161-
true,
162-
)
163-
});
164-
let (short_circuit, continuation, constant) = match op {
165-
LogicalOp::And => (else_block, then_block, false),
166-
LogicalOp::Or => (then_block, else_block, true),
151+
152+
let push_constant_bool = |this: &mut Builder<'a, 'tcx>, bb, dest, value| {
153+
this.cfg.push_assign_constant(
154+
bb,
155+
source_info,
156+
dest,
157+
ConstOperand {
158+
span: expr.span,
159+
user_ty: None,
160+
const_: Const::from_bool(this.tcx, value),
161+
},
162+
);
167163
};
168-
// At this point, the control flow splits into a short-circuiting path
169-
// and a continuation path.
170-
// - If the operator is `&&`, passing `lhs` leads to continuation of evaluation on `rhs`;
171-
// failing it leads to the short-circuting path which assigns `false` to the place.
172-
// - If the operator is `||`, failing `lhs` leads to continuation of evaluation on `rhs`;
173-
// passing it leads to the short-circuting path which assigns `true` to the place.
174-
this.cfg.push_assign_constant(
175-
short_circuit,
176-
source_info,
177-
destination,
178-
ConstOperand {
179-
span: expr.span,
180-
user_ty: None,
181-
const_: Const::from_bool(this.tcx, constant),
182-
},
183-
);
184-
let rhs = unpack!(this.expr_into_dest(destination, continuation, rhs));
164+
// A simple optimization on boolean expression with short-circuit
165+
// operators is to not create a branch for the last operand.
166+
// Example:
167+
// let x: bool = a && b;
168+
// would be compiled into something semantically closer to
169+
// let x = if a { b } else { false };
170+
// rather than
171+
// let x = if a && b { true } else { false };
172+
//
173+
// In case `a` is true, evaluate `b` and assign it to `x`,
174+
// thus there is no need to create an actual branch for `b`.
175+
// Otherwise, assign false to `x`.
176+
//
177+
// The exception is when we instrument the code for condition coverage,
178+
// which tracks the outcome of all operands of boolean expressions.
179+
180+
let (outcome1, outcome2) = if this.tcx.sess.instrument_coverage_condition() {
181+
// We first evaluate the left-hand side of the predicate ...
182+
let (then_block, else_block) =
183+
this.in_if_then_scope(condition_scope, expr.span, |this| {
184+
this.then_else_break(
185+
block,
186+
expr_id,
187+
Some(condition_scope), // Temp scope
188+
source_info,
189+
// This flag controls how inner `let` expressions are lowered,
190+
// but either way there shouldn't be any of those in here.
191+
true,
192+
)
193+
});
194+
195+
// Write true on expression success...
196+
push_constant_bool(this, then_block, destination, true);
197+
// ...and false on failure.
198+
push_constant_bool(this, else_block, destination, false);
199+
200+
(then_block, else_block)
201+
} else {
202+
// We first evaluate the left-hand side of the predicate ...
203+
let (then_block, else_block) =
204+
this.in_if_then_scope(condition_scope, expr.span, |this| {
205+
this.then_else_break(
206+
block,
207+
lhs,
208+
Some(condition_scope), // Temp scope
209+
source_info,
210+
// This flag controls how inner `let` expressions are lowered,
211+
// but either way there shouldn't be any of those in here.
212+
true,
213+
)
214+
});
215+
let (short_circuit, continuation, constant) = match op {
216+
LogicalOp::And => (else_block, then_block, false),
217+
LogicalOp::Or => (then_block, else_block, true),
218+
};
219+
// At this point, the control flow splits into a short-circuiting path
220+
// and a continuation path.
221+
// - If the operator is `&&`, passing `lhs` leads to continuation of evaluation on `rhs`;
222+
// failing it leads to the short-circuting path which assigns `false` to the place.
223+
// - If the operator is `||`, failing `lhs` leads to continuation of evaluation on `rhs`;
224+
// passing it leads to the short-circuting path which assigns `true` to the place.
225+
push_constant_bool(this, short_circuit, destination, constant);
226+
227+
let rhs = unpack!(this.expr_into_dest(destination, continuation, rhs));
228+
229+
(short_circuit, rhs)
230+
};
231+
185232
let target = this.cfg.start_new_block();
186-
this.cfg.goto(rhs, source_info, target);
187-
this.cfg.goto(short_circuit, source_info, target);
233+
this.cfg.goto(outcome1, source_info, target);
234+
this.cfg.goto(outcome2, source_info, target);
188235
target.unit()
189236
}
190237
ExprKind::Loop { body } => {

0 commit comments

Comments
 (0)